Ig 016 serializable diagnostics and config containers

[joshmann-inversionsemi]

Summary

A Simulation can now carry diagnostics (analysis that runs automatically and saves its results with the config) and parameters (curated input metadata). Both are serializable like the other config objects. A new ConfigContainer may be used to group them.

What's new

• Parameters (datapoint.py): stores a plain dict of user inputs as metadata. The simulation never interprets it.
• _Diagnostic (diagnostics.py): abstract base class. Subclasses implement _analyze() -> dict.
  • analyze() stores the results in data and sets analysis_complete. Analysis never runs during construction, loading, or serialization.
  • Setting RUN_BEFORE_SIMULATION on a subclass runs it at the end of setup. The default is after the full simulation run.
• _ParticleDiagnostic: base class for diagnostics that need particles. load_particles() reads the highest-numbered data########.h5 dump at analysis time.
  • selection takes ("elec_name" | "ion_name", name or names) or ("all_of_species", species). "e" means all electrons.
• MomentDescriptorDiagnostic: the first concrete diagnostic. It wraps compute_moment_descriptor and defaults to the 33-feature spline descriptor.
• ConfigContainer (config_container.py): an ordered, nestable list of configs. Entries can be objects, dicts, YAML/JSON strings, or file paths.
• Simulation accepts all of the above in elements. It exposes them as sim.diagnostics and sim.parameters, and runs diagnostics on the write rank only.

Usage

This is from the ionization demo (run_simulation.py):

physical_parameters = Parameters(data={"laser_energy_J": LASER_ENERGY, ...})
beam_moments = MomentDescriptorDiagnostic(selection=("all_of_species", "e"))
results = ConfigContainer(configs=[physical_parameters, beam_moments])

sim = Simulation(elements=[hyparams, laser, background_profile, dopant_profile, results])
sim.setup_simulation(working_directory=RUN_DIR)
sim.run_simulation()                      # beam_moments.analyze() runs after stepping

if beam_moments.analysis_complete:
    results.to_hdf5_file("results.h5", overwrite=True)   # inputs + results in one file

Behavior notes

• If a diagnostic's analysis raises, the error propagates. A failed pre-simulation diagnostic leaves is_setup false, and a failed post-simulation diagnostic records no completion hash.
• SerializableConfig fixes:
  • Relative paths inside a nested config file now resolve against that file's own directory.
  • from_any no longer swallows every OSError. Only ENAMETOOLONG and EINVAL fall back to parsing the string as a dict.
• Docs: README_LIB.md and the .pyi stubs are updated. The demo and its README show the new usage.

[claude]

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

[joshmann-inversionsemi]

@claude , please review this PR.

[claude]

Claude finished @joshmann-inversionsemi's task in 2m 23s —— View job

---

PR review: serializable diagnostics and config containers

• Read context (the only CLAUDE.md is under geecs-lume-twin/, so it doesn't apply here)
• Review diagnostics.py, datapoint.py, config_container.py
• Review simulation.py and serializable_config.py changes
• Review tests, stubs, docs, demo
• Post review

Overall the design is solid. Diagnostics are kept out of config_hash(), completion is runtime-only, and attachment is excluded from serialization. Switching the debug log from to_dict() to %r avoids triggering the "incomplete" warning, and the from_any OSError narrowing is a good fix. Tests cover most of the lifecycle. I didn't run the test suite, so everything below comes from reading the code. My concerns are mostly about how diagnostics interact with the run-skipping logic and with old output on disk.

---

1. Diagnostics never run when a run is skipped as already done, so the demo silently writes no results

setup_simulation() and run_simulation() both return early when _is_hashed() is true (simulation.py:671, simulation.py:917). That return happens before _analyze_diagnostics(). If a run finished but the results weren't saved (for example the process was killed after _save_hash(), or a new diagnostic was added to a finished run), then:

• beam_moments.analysis_complete stays False;
• the demo's if beam_moments.analysis_complete: guard skips results.to_hdf5_file(...) without any message.

Diagnostics aren't part of the hash, and post-run diagnostics only read output files already on disk. So they could still run when stepping is skipped. Two options: run post-run diagnostics in the skipped branch, or document the behavior and make the demo log a warning when analysis_complete is false.

Fix this →

2. A failed analysis forces the whole simulation to run again

run_simulation() calls _analyze_diagnostics() before _save_hash() (simulation.py:964-968). An exception in _analyze() therefore means no hash is recorded. On the next attempt the whole FBPIC run repeats, even though the simulation output is complete and the diagnostic isn't part of the hash. This is documented, but it's an expensive way to fail. Unless that's intended, consider saving the hash first and then running the analysis. Combined with fix #1, a re-run would then only repeat the analysis.

3. load_particles() can read old files left by an earlier run

load_particles() (diagnostics.py:230-241) picks the highest-numbered data*.h5 in <save_directory>/hdf5. Nothing clears that directory when the configuration changes. Suppose the user changes T_interact, number_dumps, or dt, or switches between boosted and unboosted runs, and keeps the same save_directory. In the unboosted case the file numbers are iteration numbers, so a file from a longer earlier run can outrank the new final dump. The diagnostic then analyzes the wrong data without any error. Some ways to fix it:

• select the file that matches the expected final iteration or dump index from this simulation;
• only accept files modified after setup_simulation();
• at least log which file was loaded.

4. Boosted frame: check that the highest-numbered dump is complete

The demo uses gamma_boost=2, so particles are written by BackTransformedParticleDiagnostic. As far as I remember, FBPIC creates the lab-frame snapshot files up front and fills them in as the boosted window passes. If so, the highest-numbered file could exist but be partly empty when stepping ends. The nextafter change to dt_lab_diag_period suggests the last snapshot is meant to line up with the end of the run. Still, please confirm that data{number_dumps-1}.h5 is fully populated at the end of a boosted run, or pick the newest complete snapshot. I couldn't check this against the FBPIC source in this environment.

5. tests/test_lib/test_dataset.py duplicates test_datapoint.py

test_dataset.py (306 lines) is a line-for-line copy of the first ~300 lines of test_datapoint.py: the same fixture and the same tests from test_parameters_retain_live_dictionary to test_diagnostics_defaults_and_examples_.... It looks like a leftover from a rename, since there is no dataset module. Every test runs twice, and the copies will drift apart. I suggest deleting it.

Fix this →

Minor / nits

• The demo logs an "incomplete" warning on every run. CONFIG_TYPE = "h5" is the default, and sim.to_hdf5_file(...) runs before the simulation. It serializes beam_moments with no results yet, which logs MomentDescriptorDiagnostic analysis is incomplete.... That's harmless but alarming for new users. Options: save cfgs/ without the results container, or add a way to suppress the warning for intentional pre-run snapshots.
• Dead path handling for containers in _sort_component (simulation.py:422-434). ConfigContainer.__attrs_post_init__ already turns every entry into an object. The second from_any(source, relative_to=...) call is therefore just a pass-through, and the relative_to computation never has any effect. This also means a relative path string inside a ConfigContainer built in Python resolves against the cwd, not the Simulation's _element_config_anchor(). If that's intended, the extra code can go. If not, resolution needs to be deferred.
• _selected_recordings for elec_name (diagnostics.py:208-212): the available set can contain None from densities without an elec_name. It's harmless, but if getattr(density, kind) would be clearer than the kind == "elec_name" or density.species is not None condition.
• Pre-simulation _ParticleDiagnostics: a _ParticleDiagnostic subclass with RUN_BEFORE_SIMULATION = True would read leftover dumps from an earlier run, or raise. You could reject that combination in _sort_component, or note it in the docstring.
• from_any with ENOENT/EACCES: I found no realistic YAML or JSON string that gets past _resolve_path's is_file() checks and then raises a different errno. The narrowing looks safe.

---

None of these block merging, except that #3 (and possibly #4) can silently produce wrong results in the demo's main workflow. #1 and #5 are quick fixes.

[joshmann-inversionsemi]

I have addressed some of the major concerns; please provide another review @claude

[claude]

Claude finished @joshmann-inversionsemi's task in 1m 2s —— View job

---

PR re-review: serializable diagnostics and config containers

• Review the three new commits (889e174, e398172, 35706eb) against the earlier findings
• Re-read simulation.py, diagnostics.py, config_container.py, serializable_config.py
• Check the tests and the demo
• Post the review

I couldn't run pytest because the command wasn't in my allowed tools, so this review comes from reading the code only.

---

Status of the earlier findings

#	Finding	Status
1	Diagnostics never run when a hashed run is skipped	✅ Fixed. Both skip branches now call _analyze_diagnostics(...) (simulation.py:675, simulation.py:924). This is covered by test_hashed_phase_reruns_analysis_without_simulation_work and test_run_skip_precedes_setup_prerequisite.
2	A failed analysis forces the whole simulation to run again	✅ Fixed. _save_hash() now runs before the post-run analysis (simulation.py:969-972). test_analysis_failure_preserves_successful_stepping_hash and test_hash_save_failure_does_not_start_analysis both cover it.
3	load_particles() can read old files from an earlier run	❌ Still open. Fix #1 makes it more likely to happen; see below.
4	Boosted frame: the highest-numbered dump may not be fully written	❌ Still open. It still needs to be checked against FBPIC's BackTransformedParticleDiagnostic.
5	Duplicate test_dataset.py	✅ Fixed in e398172.
nit	Dead path handling for containers in _sort_component	✅ Fixed. The re-resolution is gone and unresolved entries now raise TypeError (simulation.py:424-434). The docstring now says that relative paths in a container built in Python resolve against the cwd at construction time. The new tests at test_simulation.py:427 and :465 cover this.

The control flow for #1 and #2 looks right to me. Setup that is skipped as already done returns with is_setup false. run_simulation() checks the hash before it checks is_setup, so the normal demo sequence (setup skipped, run skipped, post-run analysis re-runs) works without starting FBPIC.

---

Remaining concerns

A. load_particles() still picks the highest-numbered file, and fix #1 makes that riskier (diagnostics.py:230-241)

Post-run analysis now also runs when a run is skipped. The skip only checks the hash file, and nothing checks which files are in hdf5/. For example:

1. Run config A with a long T_interact. It writes data00004000.h5.
2. Change to config B with a shorter run and the same save_directory. It writes up to data00002000.h5 and overwrites the hash with B's.
3. Run B again. Stepping is skipped, the analysis re-runs, and max() picks A's data00004000.h5.

The same thing happens on the first run of B (step 2), before any skip. This is the one issue left that can give wrong results with no error. The cheapest fix is to compute the expected final file name from the simulation and require that it exists. In the unboosted case that is the iteration number of the last dump, and in the boosted case it is number_dumps - 1. At minimum, log the path that was loaded.

Fix this →

B. Boosted-frame snapshot completeness (finding #4 from the first review, unchanged)

I still haven't checked whether the highest-numbered back-transformed snapshot is fully written when a gamma_boost=2 run ends. If FBPIC creates the lab-frame files up front, both the current code and the fix for A could read a file that is only partly filled. A short check against one real demo run would settle this, for example comparing the particle count in data{N-1}.h5 with data{N-2}.h5.

---

Nits

• The demo's else branch can no longer run, and its message is now wrong (run_simulation.py:186-189). Skipped runs re-run the analysis, and analysis errors propagate before this point. That means beam_moments.analysis_complete is effectively always True on rank 0 here. The text "This could be due to the simulation skipping its run" is no longer accurate. Either remove the branch or make it a plain safety-net message.
• working_directory default is fixed at import time (simulation.py:301). This problem predates the PR. attrs.field(default=Path.cwd(), ...) reads the cwd once, when the module is imported. Now that run_simulation() can run analysis without setup_simulation(), the hash path and load_particles() use that import-time cwd if the user never calls setup. default=attrs.Factory(Path.cwd) would be safer, though calling setup_simulation(working_directory=...) first, as the demo does, avoids the problem.
• The demo still logs an "incomplete" warning on every run. The sim.to_hdf5_file(...) call at run_simulation.py:171 serializes beam_moments before any analysis has run. This is unchanged from the first review, and it's only cosmetic.
• _selected_recordings (diagnostics.py:208-212): the kind == "elec_name" or density.species is not None condition is unchanged from the first review. if getattr(density, kind) would be clearer.

---

#1, #2, #5 and the container nit from the first review are all fixed, and they have tests. The main thing left before merging is A, possibly together with B, because it can silently analyze the wrong dump. Everything else is minor.

[joshmann-inversionsemi]

The first issue is not a problem; overwriting old runs is not supported.
The final frame should be filled in, even for boosted sims.
Otherwise, fixed a few nits.

One last check, @claude

[claude]

Claude finished @joshmann-inversionsemi's task in 51s —— View job

---

Final check: serializable diagnostics and config containers

• Review the new commits since the last review (8973e70, 2f634ff, merge acdc2a6)
• Check that the merge with main (lasy laser, Ig 010 lasy laser config class #19) integrates cleanly
• Check CI
• Post the final review

No blocking issues. This looks ready to merge.

---

Your responses to the open items

Item	Status
A. load_particles() picks up old dumps	✅ Accepted as out of scope, since overwriting old runs isn't supported. It might be worth one sentence in README_LIB.md or the _ParticleDiagnostic docstring saying each config needs a fresh save_directory, so new users don't hit this.
B. Last boosted snapshot not fully written	✅ Accepted on your confirmation that the final frame is fully written.
The demo's else branch could never run	✅ Fixed in 8973e70. results.to_hdf5_file(...) now runs unconditionally on rank 0. Analysis errors propagate before this line, so it's correct.
_selected_recordings condition	✅ Fixed in 2f634ff. if getattr(density, kind) also keeps None out of available.

Merge with main (acdc2a6)

The merge looks clean. In simulation.py, the PR's changes (container/diagnostic/parameter sorting, _analyze_diagnostics, the changes to the skip branches) sit on top of the lasy laser changes with no leftover conflicts. _DERIVED_YAML_KEYS is still exported from laser.py:55. The _LIB_MODULE_STEMS additions in _doc_management/index.py don't overlap with the lasy changes. CI pytest passes on the merged head. I couldn't run pytest locally because the command needs approval in my tool permissions, so I'm relying on the CI result.

Remaining nits (optional, none block merging)

• working_directory default is fixed at import time (simulation.py:301). attrs.field(default=Path.cwd(), init=False) reads the cwd once, when the module is imported. Now that run_simulation() can run post-run analysis on a skipped run without calling setup_simulation(), a user who never calls setup gets the hash path and load_particles() resolved against the import-time cwd. default=attrs.Factory(Path.cwd) is a one-line fix. This predates the PR, and the demo isn't affected because it calls setup_simulation(working_directory=...) first.
• The demo still logs an "incomplete" warning. The pre-run sim.to_hdf5_file(...) serializes beam_moments before any analysis, which triggers the warning at diagnostics.py:129. It's only cosmetic.

---

All earlier blocking findings are now either fixed with tests or out of scope by design. Nice work on this one.

[joshmann-inversionsemi]

@cedoss-inversion , ready for artisanal review!

[cedoss-inversion]

Just some questions on how I would do some FBPIC tasks in practice. Otherwise the demo looks great and would definitely want something like this for the bulk simulation set