Remove units for machine readability #125

asmacdo · 2024-08-12T22:35:42Z

Fixes #116
Fixes #128

Note: I put KiB into the field names of the execution summary, because ps uses KiB, this seemed cleaner than modifying the values into bytes.

$ duct -p ZZZZ_ --r-i 0.1 --s-i 0.01 --clobber sleep 1

duct is executing sleep 1...
Log files will be written to ZZZZ_
Exit Code: 0
Command: sleep 1
Log files location: ZZZZ_
Wall Clock Time: 1.001 sec
Memory Peak Usage (RSS) KiB: 1600
Memory Average Usage (RSS) KiB: 1561.905
Virtual Memory Peak Usage (VSZ) KiB: 221584
Virtual Memory Average Usage (VSZ) KiB: 216308.190
Memory Peak Percentage: 0.0%
Memory Average Percentage: 0.000
CPU Peak Usage: 0.0
Average CPU Usage: 0.000
Samples Collected: 42
Reports Written: 11

$ cat ZZZZ_info.json|jq

{
  "command": "sleep 1",
  "system": {
    "uid": "austin",
    "memory_total": 33336778752,
    "cpu_total": 20
  },
  "env": {},
  "gpu": null,
  "duct_version": "0.1.1",
  "execution_summary": {
    "exit_code": 0,
    "command": "sleep 1",
    "logs_prefix": "ZZZZ_",
    "wall_clock_time": "1.001",
    "peak_rss_kib": "1760",
    "average_rss_kib": "1760.000",
    "peak_vsz_kib": "221584",
    "average_vsz_kib": "221584.000",
    "peak_pmem": "0.0%",
    "average_pmem": "0.000",
    "peak_pcpu": "0.0",
    "average_pcpu": "0.000",
    "num_samples": 41,
    "num_reports": 10
  }
}

con#116 Note: I put KiB into the field names of the execution summary, because `ps` uses KiB, this seemed cleaner than modifying the values into bytes. ``` $ duct -p ZZZZ_ --r-i 0.1 --s-i 0.01 --clobber sleep 1 duct is executing sleep 1... Log files will be written to ZZZZ_ Exit Code: 0 Command: sleep 1 Log files location: ZZZZ_ Wall Clock Time: 1.001 sec Memory Peak Usage (RSS) KiB: 1600 Memory Average Usage (RSS) KiB: 1561.905 Virtual Memory Peak Usage (VSZ) KiB: 221584 Virtual Memory Average Usage (VSZ) KiB: 216308.190 Memory Peak Percentage: 0.0% Memory Average Percentage: 0.000 CPU Peak Usage: 0.0 Average CPU Usage: 0.000 Samples Collected: 42 Reports Written: 11 $ cat ZZZZ_info.json|jq { "command": "sleep 1", "system": { "uid": "austin", "memory_total": 33336778752, "cpu_total": 20 }, "env": {}, "gpu": null, "duct_version": "0.1.1", "execution_summary": { "exit_code": 0, "command": "sleep 1", "logs_prefix": "ZZZZ_", "wall_clock_time": "1.001", "peak_rss_kib": "1760", "average_rss_kib": "1760.000", "peak_vsz_kib": "221584", "average_vsz_kib": "221584.000", "peak_pmem": "0.0%", "average_pmem": "0.000", "peak_pcpu": "0.0", "average_pcpu": "0.000", "num_samples": 41, "num_reports": 10 } } ```

=== Do not change lines below === { "chain": [], "cmd": "./.update-readme-help.py", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

codecov · 2024-08-12T22:41:17Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.76%. Comparing base (f22b46e) to head (ea0d04e).
Report is 23 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #125      +/-   ##
==========================================
+ Coverage   93.60%   93.76%   +0.16%     
==========================================
  Files           2        2              
  Lines         344      353       +9     
  Branches       56       57       +1     
==========================================
+ Hits          322      331       +9     
  Misses         16       16              
  Partials        6        6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

asmacdo · 2024-08-12T22:46:25Z

@yarikoptic it kinda bothers me that "memory_total" is bytes, but the samples are KiB, but its fine. WDYT

src/con_duct/__main__.py

$ duct -p ZZZZ_ --r-i 0.1 --s-i 0.01 --clobber sleep 1 duct is executing sleep 1... Log files will be written to ZZZZ_ Exit Code: 0 Command: sleep 1 Log files location: ZZZZ_ Wall Clock Time: 1.006 sec Memory Peak Usage (RSS): 1802240 Memory Average Usage (RSS): 1758282 Virtual Memory Peak Usage (VSZ): 226902016 Virtual Memory Average Usage (VSZ): 221367820 Memory Peak Percentage: 0.0 Memory Average Percentage: 0.000 CPU Peak Usage: 0.0 Average CPU Usage: 0.000 Samples Collected: 41 Reports Written: 11 $ cat ZZZZ_info.json| jq { "command": "sleep 1", "system": { "uid": "austin", "memory_total": 33336778752, "cpu_total": 20 }, "env": {}, "gpu": null, "duct_version": "0.1.1", "execution_summary": { "exit_code": 0, "command": "sleep 1", "logs_prefix": "ZZZZ_", "wall_clock_time": "1.006", "peak_rss": "1802240", "average_rss": "1758282", "peak_vsz": "226902016", "average_vsz": "221367820", "peak_pmem": "0.0", "average_pmem": "0.000", "peak_pcpu": "0.0", "average_pcpu": "0.000", "num_samples": 41, "num_reports": 11 } }

=== Do not change lines below === { "chain": [], "cmd": "./.update-readme-help.py", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

src/con_duct/__main__.py

yarikoptic · 2024-08-12T23:45:54Z

Didn't forget to add a test

asmacdo · 2024-08-13T01:05:48Z

@yarikoptic I'm not sure what kind of test you had in mind, since there are other places earlier in the code that would go wrong if those values weren't numbers.

I figured it would be best to just add some more assertions into the runtime, and add validation where we first get the values from ps.

lmk if this isn't what you had in mind.

asmacdo · 2024-08-13T01:12:08Z

@yarikoptic done pushing ready for re-review

src/con_duct/__main__.py

test/test_report.py

yarikoptic

left individual comments, please reduce code duplication and possibly return original numerics in the record?

This is intended to be machine readable, better to keep the values as they are without rounding or truncation.

=== Do not change lines below === { "chain": [], "cmd": "./.update-readme-help.py", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

Accidentally left out of new venv

=== Do not change lines below === { "chain": [], "cmd": "./.update-readme-help.py", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

Removed check for timestamp being a string, and relax requirement for int rather than float for some fields. This significantly reduces complexity.

yarikoptic

minor recommendation on clearer variable naming and one reference for that "default" (now is also a separate issue so could be done separately).

src/con_duct/__main__.py

yarikoptic · 2024-08-14T15:58:18Z

src/con_duct/__main__.py

@@ -158,6 +169,9 @@ class Averages:
    num_samples: int = 0

    def update(self: Averages, other: Sample) -> None:
+        assert_num(
+            other.total_rss, other.total_vsz, other.total_pmem, other.total_pcpu
+        )


this smells like location to add a single if statement and go to assignment upon not num_samples and otherwise use += and then have defaults to None above to the averages which we simply don't know upon the moment of this class instance initiation.

Going to address this in the next one, dropping the release tag here, will add over there.

asmacdo added semver-minor Increment the minor version when merged release Create a release when this pr is merged labels Aug 12, 2024

asmacdo requested a review from yarikoptic August 12, 2024 22:35

[DATALAD RUNCMD] Autoupdate reademe

ba49b5c

=== Do not change lines below === { "chain": [], "cmd": "./.update-readme-help.py", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

yarikoptic reviewed Aug 12, 2024

View reviewed changes

src/con_duct/__main__.py Outdated Show resolved Hide resolved

asmacdo added 2 commits August 12, 2024 18:03

[DATALAD RUNCMD] Autoupdate readme

702ee47

=== Do not change lines below === { "chain": [], "cmd": "./.update-readme-help.py", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

asmacdo commented Aug 12, 2024

View reviewed changes

src/con_duct/__main__.py Outdated Show resolved Hide resolved

asmacdo added 5 commits August 12, 2024 19:18

fixup comment

15f33a9

Add ProcessStats validation

a015715

fixup sample total field, no longer kib

e835663

Ensure averages stay numbers

b548c5a

hit all the fields for coverage

39fc233

Add ts type validation

cc3622d

asmacdo marked this pull request as draft August 13, 2024 13:44

asmacdo marked this pull request as ready for review August 13, 2024 13:45

yarikoptic reviewed Aug 13, 2024

View reviewed changes

src/con_duct/__main__.py Outdated Show resolved Hide resolved

yarikoptic reviewed Aug 13, 2024

View reviewed changes

src/con_duct/__main__.py Outdated Show resolved Hide resolved

yarikoptic reviewed Aug 13, 2024

View reviewed changes

test/test_report.py Outdated Show resolved Hide resolved

yarikoptic requested changes Aug 13, 2024

View reviewed changes

yarikoptic mentioned this pull request Aug 13, 2024

Duplicate "sec sec" in summary report #128

Closed

asmacdo added 2 commits August 13, 2024 14:01

execution summary file should contain raw values

098a581

This is intended to be machine readable, better to keep the values as they are without rounding or truncation.

Add rounding and units for user-facing (rendered) summary

48e1794

asmacdo marked this pull request as draft August 13, 2024 19:12

asmacdo added 7 commits August 13, 2024 14:13

[DATALAD RUNCMD] Autoupdate readme

bac8087

=== Do not change lines below === { "chain": [], "cmd": "./.update-readme-help.py", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

type check func

417e493

fixup rm execution formatting, must be ok with 'unknown'

9c29b60

fix map usage

4b5a289

fixup mypy

7c211f0

rerun pre-commit -a

36222fa

Accidentally left out of new venv

shorten line, ignored by black, but flake8 failed

774227c

asmacdo force-pushed the machine-readable-output branch from 0375f45 to 774227c Compare August 13, 2024 20:50

asmacdo added 3 commits August 13, 2024 15:51

[DATALAD RUNCMD] Autoupdate readme

8dd4d71

=== Do not change lines below === { "chain": [], "cmd": "./.update-readme-help.py", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

Consolodate number checking

3f9099d

Removed check for timestamp being a string, and relax requirement for int rather than float for some fields. This significantly reduces complexity.

consolidate and parameterize tests

3af906a

asmacdo marked this pull request as ready for review August 13, 2024 21:31

asmacdo requested a review from yarikoptic August 13, 2024 21:41

yarikoptic requested changes Aug 14, 2024

View reviewed changes

Explicitly show that ps gives memory in KiB

ea0d04e

asmacdo removed the release Create a release when this pr is merged label Aug 14, 2024

asmacdo merged commit c75a8cc into con:main Aug 14, 2024
30 checks passed

asmacdo deleted the machine-readable-output branch August 14, 2024 17:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove units for machine readability #125

Remove units for machine readability #125

asmacdo commented Aug 12, 2024 •

edited

Loading

codecov bot commented Aug 12, 2024 •

edited

Loading

asmacdo commented Aug 12, 2024

yarikoptic commented Aug 12, 2024

asmacdo commented Aug 13, 2024 •

edited

Loading

asmacdo commented Aug 13, 2024

yarikoptic left a comment

yarikoptic left a comment

yarikoptic Aug 14, 2024

asmacdo Aug 14, 2024

asmacdo Aug 14, 2024

Remove units for machine readability #125

Remove units for machine readability #125

Conversation

asmacdo commented Aug 12, 2024 • edited Loading

codecov bot commented Aug 12, 2024 • edited Loading

Codecov Report

asmacdo commented Aug 12, 2024

yarikoptic commented Aug 12, 2024

asmacdo commented Aug 13, 2024 • edited Loading

asmacdo commented Aug 13, 2024

yarikoptic left a comment

Choose a reason for hiding this comment

yarikoptic left a comment

Choose a reason for hiding this comment

yarikoptic Aug 14, 2024

Choose a reason for hiding this comment

asmacdo Aug 14, 2024

Choose a reason for hiding this comment

asmacdo Aug 14, 2024

Choose a reason for hiding this comment

asmacdo commented Aug 12, 2024 •

edited

Loading

codecov bot commented Aug 12, 2024 •

edited

Loading

asmacdo commented Aug 13, 2024 •

edited

Loading