Repository navigation
Context collectors - #71
Conversation
92b6bc1 to
2f2590d
Compare
8d93575 to
f125103
Compare
…ts against them A project's benchx.conf.json can now name context collectors, commands the runner runs once per order, before the cases. They run in the workers' own environment and working directory. Each prints a JSON object of facts. The runner saves it whole as an artifact and records its summary and sha256 in every result's observed_context. A target's how_built may now be an object of facts as well as text. With verify: "how_built", every declared key must be observed with exactly the same value, and a mismatch, or a collector that can't run, refuses the order before any case. on_fail: warn runs it and marks the results collector-mismatch instead. bx run refuses orders with collectors unless given --allow-collectors, because the commands are executed. The numpy-bench example uses this. provider.py declares what the numpy in the build directory reports (numpy_facts.py), refuses a build that isn't the checkout's HEAD, and the collector checks that the workers import the same numpy.
f125103 to
984d68d
Compare
|
@MaanasArora while I have your review attention :-), could I tempt you to take a look at this one next? |
There was a problem hiding this comment.
Thanks, looking good @ev-br! Two cheap validation changes, they can be done here I think!
I have one main comment, which is that bx compare may not run as-is on the NumPy example. The issue is that revision-dependent facts like numpy_version would split history.
I have some suggestions for a refactor but maybe for follow-up? I'm happy to accept that the comparison example will be addressed in follow-up, and we can merge this soon then!
| last = (done.stderr.strip().splitlines() or ["no message"])[-1] | ||
| return None, f"exit {done.returncode}: {last}" | ||
| try: | ||
| facts = json.loads(done.stdout) |
There was a problem hiding this comment.
I think we should use the validated JSON loader here, to parallel the order parsing. Then the collector fails on nan or infinity, for example.
| yield f"{path}/{key}: missing (declared {_short(value)})" | ||
| else: | ||
| yield from _differences(value, found[key], f"{path}/{key}") | ||
| elif wanted != found: |
There was a problem hiding this comment.
I think we should use core.canonical here, otherwise e.g. bools and ints would match!
| ``` | ||
|
|
||
| The driving half captures that output as a provenance artifact and adds the facts to the context document's `environment.system` under the collector's name, here `environment.system.numpy-build.blas`, with the collector's name as their source. Translation therefore stays repeatable from artifacts, and a collector cannot collide with the snapshot or with another collector. Facts are copied to every result of the invocation they were collected around. A collector that fails, times out, or prints anything else contributes no facts, and those results carry the warning `collector-failed`; nothing is substituted and the run continues. | ||
| The driving half captures that output as a provenance artifact and adds the facts to the context document's `environment.system` under the collector's name, here `environment.system.numpy-build.blas`, with the collector's name as their source. Translation therefore stays repeatable from artifacts, and a collector cannot collide with the snapshot or with another collector. Facts are copied to every result of the invocation they were collected around. A collector may also be held to declared facts (`verify`, `expect`) and refuse the order when they differ; the prototype implements this beyond the design's "nothing checks one against the other" (`prototype-design.md` §3). A collector that fails, times out, or prints anything else contributes no facts, and those results carry the warning `collector-failed`; nothing is substituted and the run continues. |
There was a problem hiding this comment.
environment.system splits history (by default), so any version change would split history in the NumPy example. In that case, it doesn't seem like revisions could actually run? (The hash also differs, not just summary.)
I did say collector facts should split history, but it seems I wasn't thinking about run verification yet! I think we should at least change this to allow putting some facts in provenance, the ones which shouldn't be coordinates. We could rename summary to environment and only put that in environment.facts?
Use a more resilient json loading/parsing in context collectors
|
Thanks @MaanasArora ! I addressed two comments about json parsing and opened #97 for your larger comment. I think it's partly addressed by #81, which builds on this PR. Let me merge this one now and rebase #81. |
One more patch from dogfooding synthetic numpy benchmarks. On top of gh-65.I'm building numpy with
spin buildin a fixed envronment in a loop (UC-03-style, only withbx benchnotcompare). Since checking out and building is in the workbench, the provider records the built numpy state (vianp.show_configbut that's configurable per bench suite). Now, I want to be able to check in the runner that the numpy I actually import is what I meant to, not some other numpy. To this end, use context collectors to use the same mechanism as the provider to query the numpy state and check it matches. If they do not, it's an error.A project's benchx.conf.json can now name context collectors, commands the runner runs
once per order, before the cases. They run in the workers' own environment and working
directory. Each prints a JSON object of facts. The runner saves it whole as an artifact
and records its summary and sha256 in every result's observed_context. A target's how_built
may now be an object of facts as well as text.
With verify: "how_built", every declared key must be observed with exactly the same value,
and a mismatch, or a collector that can't run, refuses the order before any case. on_fail:
warn runs it and marks the results collector-mismatch instead. bx run refuses orders with
collectors unless given --allow-collectors, because the commands are executed. The numpy-bench
example uses this. provider.py declares what the numpy in the build directory reports
(numpy_facts.py), refuses a build that isn't the checkout's HEAD, and the collector checks that
the workers import the same numpy.