36 orchestration v1#39
Merged
Merged
Conversation
Synchronous, in-memory orchestration following adr-2026-06-12 and adr-2026-06-19: - Orchestrator: drives runs (one parameter point per run, user writes the sweep loops), registers channels, owns the data. Fail-fast. - Metric: named write-handle channel; dtype-checked pushes with skip_if_exists; data lives on the orchestrator, not the metric. - MetricDataFrame: per-metric storage using pandas internally; parameters stored once per run and exposed as a parameter-indexed frame, sliceable by parameter values. - Current run state kept in plain module globals (no context manager, no stack), per adr-2026-06-19.
Add a Sphinx reference page for krum.orchestration (Orchestrator, Metric, MetricDataFrame) and register it in the documentation index.
Wrap the experiment call in run() so that a failure re-raises a RuntimeError identifying the run's parameters, with the original exception chained. Keeps fail-fast (the sweep still stops) while making failures diagnosable.
Each run's parameters are already encoded in its sample key (tuple(sorted(params.items()))), so the separate _params dict duplicated them. Reconstruct parameters from the key with dict(run_key) when materialising the frame, and keep a single _param_order list for column ordering (run keys are sorted, so they cannot preserve the user's parameter order). Output is unchanged.
Resolve each experiment's default arguments in run() (via the signature) so a defaulted parameter is recorded whether or not it was passed explicitly, giving runs that differ only in that respect one identity. When materialising the frame, fill a parameter a run lacks with NaN instead of raising, so runs with differing parameter sets render cleanly.
Cover Metric (creation/push guards, dtype enforcement, name validation), Orchestrator (run/get, sweep accumulation, reserved names, fail-fast wrapping, default resolution) and MetricDataFrame (params stored once, slicing, NaN fill, copy semantics).
Record the experiment on the first run() and reject a later run() that passes a different one with a clear ValueError. A run is identified only by its parameters, so allowing several experiments in one orchestrator would let their samples silently collide; use a separate orchestrator per experiment.
Cover that repeated runs of the same experiment are allowed and that running a different experiment on the same orchestrator raises ValueError.
A backport of PEP 814's builtin frozendict for the supported Python versions (3.10-3.14): an immutable, hashable Mapping with order-independent equality and hashing but order-preserving iteration. On Python 3.15+ the module aliases the builtin frozendict directly. Behaviour was verified against the real 3.15.0b1 builtin (constructor forms, equality, lazy hashing, repr, immutability).
Make the fallback mapping shallowly immutable and preserve its hash and equality invariants. Add dict-like construction from mappings and iterable pairs, generic key/value typing, union operators, shallow and deep copy behavior, pickling support, and recursive-safe representation. Expand focused tests to cover the new behavior.
Use FrozenDict parameter mappings as stable run keys and remove the separately maintained parameter-order state. Replace dataframe() with to_pandas(), materializing flat parameter columns from their ordered union and marking parameters absent from a run with pd.NA. Keep step and value as ordinary columns instead of constructing a MultiIndex. Make filtered calls return independent MetricDataFrame stores, preserving the distinction between absent parameters and parameters explicitly set to None. Update dataframe, metric, and orchestrator tests for the new API and behavior.
Remove the single-experiment binding so one orchestrator can collect metrics from multiple experiment functions. Record each run with module-qualified experiment metadata and a 12-character SHA-256 source hash, falling back to the qualified identity when source inspection is unavailable. Reserve the generated metadata column names to avoid user-parameter collisions. Update orchestrator tests for non-colliding multiple experiments, metadata columns, exact source hashing, fallback hashing, and reserved experiment metadata names.
The hash-consistency and immutability tests probed the backport's private __data attribute, which the built-in frozendict (Python 3.15+, aliased by the module) does not expose, making them fail there. Gate those probes behind a _USES_BUILTIN check while keeping the public-contract assertions on every version. Verified against the real 3.15.0b1 builtin.
Calling a MetricDataFrame now returns a narrowed MetricDataFrame, not a pandas frame; update the example comment and show to_pandas() for the pandas export.
Rename run helpers to hidden_vulnerability_experiment and krum_experiment, and invoke them via Orchestrator.run in experiment scripts. Import Orchestrator in experiments and retrieve metrics after runs. Remove aggregator_f and evaluate_fn parameters from CentralisedSimulation. Use Metric to record loss/error in krum_experiment. Implement KrumSimulation.evaluate and add an evaluate stub for HiddenVulnerabilitySimulation.
Bring the layer into compliance with the pre-commit checks: add the missing package docstring, apply ruff formatting, annotate FrozenDict's slotted attributes and type its constructor dict so ty can resolve them, and switch the intentional private-attribute probes in the tests to ty's own ignore codes.
Define evaluation by overriding CentralisedSimulation.evaluate rather than supplying a callable. Document protocol-specific metric tuples (e.g. Krum -> (test_loss, test_error); HiddenVulnerability -> (test_loss, test_error, test_accuracy)). Change eval_every to be a hint for experiment scripts and remove evaluate_fn docs. Fix metric ordering in HiddenVulnerability and add an explicit return docstring to Krum.evaluate.
Run parameters form the run's identity key, so each value must be hashable.
Recursively convert dicts to FrozenDict, lists/tuples to tuples, and sets to
frozensets before building the key, so structured parameters such as
attack_kwargs={...} are accepted (the experiment still receives the original
values). Reject any value that is unhashable even after freezing with a clear
error naming the parameter, instead of a deep TypeError. The freezing lives in
the orchestrator, not FrozenDict, so behaviour matches the 3.15 builtin.
Filtering a metric store is clearer as an explicit `.filter(**params)` method than call syntax. Update the package example and the dataframe tests; the returned store still shallow-copies each run's samples so it stays independent.
Tag each run with the experiment function itself under the reserved 'experiment' key, dropping the name + SHA-256 source-hash scheme and the 'experiment_hash' column. Update the orchestration tests accordingly.
…e=True) PyTorch 2.11+ defaults nn.Module.zero_grad() to set_to_none=True, which drops parameter .grad attributes. Because Model caches a flat gradient view that shares storage with those .grad tensors, the cached view became stale after a plain zero_grad() call: every subsequent worker returned the same outdated gradient, aggregators produced identical metrics, and training diverged. The gradients property now detects when the cached flat buffer is out of sync with the module's .grad tensors and calls relink_gradients() automatically. relink_gradients() also handles the case where the flat gradient view has never been initialized. Includes regression tests in tests/primitives/test_model.py.
- Extend HiddenVulnerability to accept aggregator_f and forward to aggregator_kwargs - Use a per-parameter RNG for Xavier initialization to preserve global RNG determinism - Pass default f and n to the aggregator when not provided - Rename MNIST F_DECLARED to MNIST_F_DECLARED_BULYAN and update references - Remove deprecated Krum CONFIGS in krum_nips_2017/experiment_3.py and standardize run signature
Clarify two Byzantine budgets: the real count f and the declared aggregator_f, and state that an explicit f in aggregator_kwargs takes precedence over aggregator_f Add a guard in CentralisedSimulation to prevent aggregator=None when f > 0 and an attack is configured, raising a clear error to avoid a scenario where Byzantine gradients have nowhere to go. When aggregator=None, compute the mean of produced gradients instead of passing through all_gradients, ensuring correct behavior when Byzantine gradients are absent
- Increase ROUNDS to 100 and set BYZ_WORKERS as round(WORKERS * 0.33) - Double WORKERS to 20 and adjust LR to 0.1 and BATCH to 8 - Add MultiKrum support and pass aggregator_kwargs configure 3 - Remove error metric plotting and related data retrieval
Collaborator
Author
|
Added simple matplotlib plotting (step vs value per metric), output saved as experiment_1.png in the run directory. |
pevab
previously requested changes
Jul 1, 2026
|
|
||
| from typing import TYPE_CHECKING, Any | ||
|
|
||
| if TYPE_CHECKING: |
Collaborator
Author
There was a problem hiding this comment.
Le if TYPE_CHECKING: évite un import circulaire : orchestrator.py importe déjà begin_run/end_run depuis _context.py, donc importer Orchestrator normalement ici créerait un cycle orchestrator.py → _context.py → orchestrator.py.
Le fichier a "from future import annotations" en haut, donc les annotations (comme orchestrator: Orchestrator) ne sont jamais évaluées à l'exécution, juste stockées comme texte. Orchestrator n'a donc pas besoin d'exister comme vrai objet au runtime — seulement pour les type checkers (on a ty dans les pre-commit hooks). TYPE_CHECKING vaut False à l'exécution (donc jamais de vrai import, pas de cycle) mais True pour ty/l'IDE, qui peuvent donc quand même vérifier les types correctement (ex: le retour de active_orchestrator() -> Orchestrator).
Collaborator
There was a problem hiding this comment.
Je trouve bizarre ces dépendances circulaires entre fichiers. Pourquoi ne pas merger _context.py dans orchestrator.py?
Metric names are only ever used as internal dict keys; nothing downstream (file paths, CLI, columns) requires them to be space-free.
FrozenDict only ever serves as a hashable dict key in this codebase, so drop the unused dict-parity methods (__or__/__ror__/__ior__, copy, __deepcopy__, __reduce__) and their tests. Verified equivalent behaviour on the Python 3.15 builtin frozendict alias path.
_context.py only existed to break a circular import between Orchestrator and its own ambient run-state functions; moving that state next to the Orchestrator class removes the cycle (and the TYPE_CHECKING workaround) entirely. Also drop the unrelated TYPE_CHECKING-only Callable import, which had no circularity to avoid in the first place.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.