Repository navigation
feat(core): versioned TOML configuration schema - #442
Conversation
Add hmf.core.config: a configuration is the unit of serialisation and reproducibility of a stage tree. It holds every field's resolved value (defaults and computed defaults included) under an [hmf] header with the schema version, the root stage class, and the versions of hmf, camb, classy, astropy, numpy and scipy. - One cattrs Converter encodes models (qualified registry name and every field), Quantities (value and unit, h-units explicit, littleh power checked through units.to_canonical on reading), arrays, astropy cosmologies (through astropy's mapping I/O), accuracy settings, mass definitions and other attrs values (with a "class" key when the class is not the declared one). - to_config/from_config, dumps_toml/loads_toml, dump_toml/load_toml. Partial hand-written configurations read through the routing table, with the defaults of build(); routing.from_paths builds a tree from resolved paths. - Schema migrations are an explicit registry (n -> n+1), logged when applied; newer or unknown schemas raise. Differing package versions are logged, or raise with strict=True. - _serialise.canonical encodes every non-plain value through the same converter. Disk-cache keys hash plain run inputs only, so they are unchanged (pinned by a test). field(physics=False) marks fields left out of content hashes (disk_cache). - Stages pickle and deep-copy as their configuration plus the Boltzmann runs already used, as plain arrays, so unpickling runs no Boltzmann code and gives bit-identical results. - A golden-file test of the default MassFunction.build() configuration enforces "default changes are schema changes"; add the "changes results" label and release-notes category, and document the policy. Fixes #391 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019U8u43VG3MHXhRTiBtYaro
Reviewer's GuideAdds a versioned, typed TOML/mapping configuration as the reproducibility boundary for hmf stage trees, integrates it with canonical content hashing and backend-aware pickling, and establishes migration, default-stability, compatibility, and result-regression tests and documentation. Sequence diagram for configuration-based stage picklingsequenceDiagram
participant Stage
participant Config as hmf.core.config
participant Boltzmann as Boltzmann memo
participant Pickle
Stage->>Config: _pickle_state(stage)
Config->>Config: to_config(stage)
Config->>Boltzmann: export_runs(computed runs)
Config-->>Pickle: configuration and keyed arrays
Pickle->>Config: _unpickle(configuration, runs)
Config->>Boltzmann: import_runs(runs)
Config->>Config: from_config(configuration)
Config-->>Pickle: rebuilt stage
Pickle-->>Stage: stage with reusable results
Sequence diagram for schema migration and version validationsequenceDiagram
participant Caller
participant Config as hmf.core.config
participant Migration as Migration registry
participant Runtime as Running packages
Caller->>Config: from_config(mapping, migrate, strict)
Config->>Config: _upgrade(mapping)
alt older schema and migration enabled
Config->>Migration: lookup MIGRATIONS[n]
Migration-->>Config: migration function
Config->>Migration: apply migration
else newer schema or migration unavailable
Config-->>Caller: ValueError
end
Config->>Runtime: package_versions()
alt versions differ and strict
Config-->>Caller: ValueError
else versions differ and non-strict
Config-->>Caller: warning, then build stage
end
Config-->>Caller: rebuilt stage
Entity relationship diagram for configuration dataerDiagram
CONFIGURATION ||--|| HEADER : contains
HEADER ||--o| PACKAGE_VERSIONS : records
CONFIGURATION ||--|| ROOT_STAGE : serializes
ROOT_STAGE ||--o{ STAGE : contains
STAGE ||--o{ MODEL : contains
CONFIGURATION {
int schema_version
string stage
}
HEADER {
int schema_version
string stage
}
PACKAGE_VERSIONS {
string hmf
string astropy
string numpy
string scipy
}
ROOT_STAGE {
string resolved_fields
}
STAGE {
string field_table
}
MODEL {
string model
string fields
}
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #442 +/- ##
==========================================
+ Coverage 99.68% 99.70% +0.02%
==========================================
Files 68 69 +1
Lines 8807 9423 +616
==========================================
+ Hits 8779 9395 +616
Misses 28 28 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Hey - I've found 12 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/hmf/core/config.py" line_range="910" />
<code_context>
+ "stage = 'hmf.core.mass_function:MassFunction'."
+ )
+ found = _import_from_path(name)
+ if not (isinstance(found, type) and issubclass(found, Stage)):
+ raise ValueError( # noqa: TRY004 (a configuration error: see hmf.core.domain)
+ f"The configuration's stage {name!r} is not a stage class."
</code_context>
<issue_to_address>
**Untrusted configs execute imported code**
When an attacker can provide a TOML configuration and name an importable module in its `stage`, `class`, or model field, `loads_toml` passes the supplied path to `_stage_class`, and `_import_from_path` imports its module before checking whether the result is a `Stage`; class and model fields use similar import paths. The module’s top-level code therefore runs in the hmf process while parsing the config.
Restrict config-driven class resolution to an explicit allowlist or registry, or require an explicit trust opt-in before importing classes from configuration data.
Also at `src/hmf/core/config.py:366`, `src/hmf/core/config.py:725`.
</issue_to_address>
### Comment 2
<location path="src/hmf/core/config.py" line_range="1029" />
<code_context>
+ for name, then in written.items():
+ now = running.get(name)
+ if name in running and (None if _is_none(then) else then) != now:
+ differ.append(f"{name} {then or 'not installed'} (now {now or 'not installed'})")
+ if not differ:
+ return
</code_context>
<issue_to_address>
**Configuration forges warning logs**
When an untrusted TOML document gives a mismatching package version containing a newline, `_check_versions` interpolates the supplied version string into its warning without escaping control characters; with the default non-strict mode, a crafted multiline value produces forged log entries that mislead log readers or parsers.
Validate version values against an expected version format or escape control characters before including them in warnings and errors.
</issue_to_address>
### Comment 3
<location path="src/hmf/core/config.py" line_range="1291" />
<code_context>
+ if descriptor is None:
+ return None
+ try:
+ return descriptor.__get__(obj, type(obj))
+ except AttributeError: # an empty cached_property slot: not computed
+ return None
</code_context>
<issue_to_address>
**Pickling runs uncached calculations**
When a stage with uncached derived properties or a solution is pickled or deep-copied, `computed` invokes cached-property descriptors for derived properties and `solution`, so an unevaluated stage runs its calculation, including CAMB or CLASS, during serialization instead of just capturing existing runs.
Check the cached-property storage without invoking the descriptor, and only collect runs from values that are already cached.
Also at `src/hmf/core/config.py:1302-1303`.
</issue_to_address>
### Comment 4
<location path="src/hmf/core/config.py" line_range="663" />
<code_context>
+ if isinstance(data, Mapping):
+ if "value" in data and "unit" in data and set(data) <= _QUANTITY_KEYS:
+ return _quantity(data, where)
+ return {k: _structure_untyped(v, f"{where}.{k}") for k, v in data.items()}
+ if isinstance(data, list):
+ return [_structure_untyped(v, where) for v in data]
</code_context>
<issue_to_address>
**Serialized values lose their types**
When untyped values, cosmology metadata, or CAMB/CLASS settings contain an encoded array or a mapping matching a None or Quantity marker, `_structure_raw` and `_structure_untyped` leave some encoded arrays as mappings or lists and mistake ordinary mappings for special markers, so configuration round trips change metadata and settings values or replace `None` with a mapping.
Make untyped decoding restore encoded arrays and special values while preserving ordinary mappings that happen to match marker shapes.
Also at `src/hmf/core/config.py:478`, `src/hmf/core/config.py:580`, `src/hmf/core/config.py:658-662`, `src/hmf/core/config.py:786`.
</issue_to_address>
### Comment 5
<location path="src/hmf/core/config.py" line_range="442" />
<code_context>
+
+
+def _plain_values(values: np.ndarray[Any, Any]) -> Any:
+ return values.item() if values.ndim == 0 else values.tolist()
+
+
</code_context>
<issue_to_address>
**Zero-dimensional arrays become scalars**
When a float64 ndarray has zero dimensions, `_plain_values` converts the array to a Python scalar, and `_structure_array` leaves that scalar unchanged, so configuration round trips return a number instead of an ndarray.
Preserve the array shape in the encoding and rebuild a zero-dimensional ndarray when structuring it.
Also at `src/hmf/core/config.py:446-448`, `src/hmf/core/config.py:649-653`.
</issue_to_address>
### Comment 6
<location path="src/hmf/core/_boltzmann.py" line_range="529" />
<code_context>
if disk_cache is not None:
disk_cache.store(key, run.to_arrays())
+ _remember(key, run)
+ return run
+
</code_context>
<issue_to_address>
**Concurrent requests repeat backend work**
When two threads request the same run key while it is absent from `_memo`, both callers miss `_memo` before either reaches `_remember`, so each runs `_RUNNERS[backend]` and increments `_counts`; the later result overwrites the earlier memo entry, and both callers receive separate runs.
Coordinate same-key cache misses so only one caller runs the backend and the others reuse its result.
</issue_to_address>
### Comment 7
<location path="src/hmf/core/_serialise.py" line_range="145-147" />
<code_context>
if isinstance(obj, (list, tuple)):
return [canonical(v) for v in obj]
- raise TypeError(f"Can't serialise a {type(obj).__name__} canonically.")
+ from . import config # imports every stage: not at import time
+
+ return canonical(config.canonical_value(obj))
</code_context>
<issue_to_address>
**Some existing cache entries miss**
When a CAMB setting accepted by the model contains a NumPy array and a disk cache contains an entry created before this change, `canonical` now encodes arrays through `config.unstructure`, which writes float64 arrays as lists instead of the previous `{"__ndarray__": ..., "dtype": ...}` form. When a CAMB setting contains a NumPy array, that setting flows into the run input and changes its key, so existing disk-cache entries for that run are missed.
Preserve the previous canonical array encoding for values used in Boltzmann run keys, or version the cache key and explicitly account for the resulting cache invalidation.
Also at `src/hmf/core/config.py:446-448`.
</issue_to_address>
### Comment 8
<location path="src/hmf/core/config.py" line_range="1206" />
<code_context>
+ The file written.
+ """
+ out = Path(path)
+ out.write_text(dumps_toml(stage, versions=versions), encoding="utf-8")
+ return out
+
</code_context>
<issue_to_address>
**Interrupted save destroys prior config**
When a process stops or a write fails while `dump_toml` is replacing an existing file, `Path.write_text` opens the destination for replacement before writing the full TOML; a process interruption or write failure leaves the previous configuration truncated or incomplete, so the saved configuration is lost or unreadable.
Write the TOML to a temporary file in the destination directory, then atomically replace the destination after the write succeeds.
</issue_to_address>
### Comment 9
<location path="src/hmf/core/config.py" line_range="786" />
<code_context>
+ float, _strict(float, (int, float, np.integer, np.floating), "a number")
+)
+converter.register_structure_hook(str, _strict(str, (str,), "a string"))
+converter.register_structure_hook_func(lambda t: True, _structure_raw)
+converter.register_structure_hook_func(lambda t: isinstance(t, type), _structure_class)
+converter.register_structure_hook_func(attrs.has, _structure_attrs)
</code_context>
<issue_to_address>
**Units cannot be round-tripped**
When a caller structures a unit encoded by `config.unstructure`, `unstructure` emits a mapping for a unit, but `structure` has no inverse hook and returns that mapping unchanged, so the caller does not get an Astropy unit.
Register a structure hook that decodes the emitted unit mapping as an Astropy unit.
Also at `src/hmf/core/config.py:526`.
</issue_to_address>
### Comment 10
<location path="tests/core/test_core_edge_cases.py" line_range="187" />
<code_context>
assert out["quantity"]["unit"] == "Mpc"
- assert "littleh" in out["unit"]["__unit__"]
+ assert "littleh" in out["unit"]["unit"]
assert out["cosmology"] == _serialise.canonical(Planck18.clone(name="x"))
- assert out["model"]["__class__"] == "hmf.core.transfer_models:BBKS"
+ assert out["model"]["model"] == "hmf.core.transfer_models:BBKS"
</code_context>
<issue_to_address>
**Cosmology hash regressions go unnoticed**
When the canonicalizer stops including cosmology parameters in content hashes, in `test_canonical_handles_every_supported_type` (`tests/core/test_core_edge_cases.py:187`), comparing Planck18 with a clone that only changes its name passes even if canonicalization drops all cosmology parameters. `test_content_hash_is_stable_and_physics_only` (`tests/core/test_core_config.py:504`) also changes `n_s` and `sigma_8` alongside the cosmology, so the hash can differ without including cosmology physics.
Assert that canonical values or hashes differ for otherwise-identical cosmologies with different physical parameters, while still ignoring name and metadata.
Also at `tests/core/test_core_config.py:504`.
</issue_to_address>
### Comment 11
<location path="src/hmf/core/config.py" line_range="486-506" />
<code_context>
+
+
+def _unstructure_attrs(value: Any) -> dict[str, Any]:
+ """Encode an attrs value (a model, stage, setting, ...) as a table of its fields."""
+ cls = type(value)
+ mode = _mode.get()
+ out: dict[str, Any] = {}
+ if isinstance(value, Model):
+ out["model"] = qualified_name(cls)
+ for key, a in _fields(cls).items():
+ if mode == "hash" and not is_physics(a):
+ continue
+ if key in out: # pragma: no cover - no model has a field named "model"
+ raise TypeError(
+ f"{cls.__name__}: the field {key!r} can't be written in a configuration."
+ )
+ field_value = getattr(value, a.name)
+ if field_value is None:
+ if mode == "toml" and a.default is None:
+ continue # left out: reading it back gives the default, None
+ out[key] = _none()
+ else:
+ out[key] = _unstructure_field(field_value, a.type)
+ return out
+
+
</code_context>
<issue_to_address>
**Accuracy table layout**
The issue asks for accuracy settings under an `accuracy` table per stage. The change serializes them as individual fields in the stage tree instead, so the requested table layout is missing.
</issue_to_address>
### Comment 12
<location path="tests/core/test_core_config.py" line_range="159-164" />
<code_context>
+ assert changed != model
+ variants.append(changed)
+ for value in variants:
+ data = config.unstructure(value)
+ assert data["model"] == name
+ assert config.structure(data, kind) == value
+ assert config.unstructure(config.structure(data, kind)) == data
+ roundtrip(_holder(kind, value))
+
+
+def test_a_model_by_alias_or_partial_table():
</code_context>
<issue_to_address>
**Some model fields can disappear unnoticed**
When the serializer omits a model field that the tested instance leaves at its default, when unstructuring omits `BBKS.b`, `test_every_registered_model_round_trips` (`tests/core/test_core_config.py:159–163`) exercises only the first changed field: `_changed()` varies `a` and leaves `b` at its default. Structuring restores that default, so the equality and self-round-trip assertions pass even though a configured non-default `b` would be lost.
Assert that each model's encoded data contains every expected field, and exercise non-default values for each field rather than only the first eligible one.
</issue_to_address>Linked issues
#391 [v4] Versioned configuration schema; serialise configs, not live objects · 7 of 8 met
| # | Requirement | Status | Notes |
|---|---|---|---|
| 1 | Store resolved stage values with a schema version and package versions. | ✅ Met | The configuration serializes stage fields and adds schema, root-stage, and package-version data in the [hmf] header. |
| 2 | Serialize and load stage configurations as TOML, including Quantities with explicit units and littleh validation. | ✅ Met | The TOML APIs use the converter, and Quantity loading parses littleh-enabled units and checks their littleh power. |
| 3 | Include accuracy settings under an accuracy table per stage. | 🟡 Partly met | Accuracy settings are serialized, but as individual stage fields such as mass_accuracy and linear_power.transfer.k_accuracy, not under an accuracy table. |
| 4 | Round-trip registered models and accuracy settings, including Quantities with littleh. | ✅ Met | The tests exercise model and accuracy round trips and littleh Quantity serialization. |
| 5 | Migrate older schemas with a logged note or raise a clear error. | ✅ Met | Older configurations use registered migrations with INFO logging; absent migrations and newer schemas raise ValueError. |
| 6 | Hash physics and accuracy fields with the relevant package versions. | ✅ Met | The content hash uses physics fields and the hashed package versions; accuracy fields remain physics fields by default. |
| 7 | Pickle configurations plus an optional cache, never live Boltzmann objects, and preserve results bit for bit. | ✅ Met | Stages pickle their configuration and used Boltzmann runs as arrays; tests check that unpickling avoids reruns and preserves results. |
| 8 | Announce physics-default changes in release notes under Changes results. | ✅ Met | The change adds the Changes results label and release-drafter category, and the golden-file test enforces the default-change policy. |
Sourcery assessment
Needs a human reviewer. 12 findings to address first, and if the new encoder or decoder records or rebuilds a field incorrectly, saved TOML configurations or pickles can produce different calculation results; the changed canonical form can also leave old cache entries unused. Those effects are bounded and can be corrected or recomputed, but reverting does not undo artifacts already written.
Blocking findings: src/hmf/core/config.py:910, src/hmf/core/config.py:1029, src/hmf/core/config.py:1291, src/hmf/core/config.py:663, src/hmf/core/config.py:442, and 7 more
CI installs classy, so the not-installed branch of package_versions never ran there (codecov/patch: 2 lines). Force it with a patched importlib.metadata. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019U8u43VG3MHXhRTiBtYaro
- Quote package versions in the version-mismatch message, so a crafted
version can't fake a log line.
- dump_toml writes a temporary file and renames it, so an existing file is
replaced whole or not at all.
- Structure units ({unit = "..."} or a string) and 0-d arrays back to their
types.
- Document that reading a configuration imports the classes it names.
- Tests: every model field is written; each cosmology parameter alone
changes the content hash.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019U8u43VG3MHXhRTiBtYaro
Summary
Adds
hmf.core.config, which makes a configuration the unit of serialisation and reproducibility of a v4 stage tree. A configuration holds every field's resolved value, including defaults and computed defaults such as σ8 and n_s from the cosmology. It sits under an[hmf]header with:Stages pickle as their configuration, and
_serialise.canonical/content_hashencode values through the same cattrs converter.API:
to_config(stage)andfrom_config(mapping, *, migrate=True, strict=False, stage=None);dumps_toml/loads_toml, anddump_toml(stage, path)/load_toml(path);content_hash(stage)andregister_migration;unstructure/structurefor single values.Schema layout
The configuration mirrors the stage tree's fields, with each stage held as a field written as a table. Derived stages (
LinearPower.growth,MassFunction.variance) and cached results are not written; they are rebuilt.Below is the full default configuration,
config.dumps_toml(MassFunction.build()). It is the checked-in golden file plus[hmf.versions], with versions from this dev checkout:disk_cache,mass_definitionandmass_conversionareNoneby default. TOML leaves them out; the mapping keeps them asNone.A partial, hand-written configuration also loads, using the names
from_flataccepts: nested tables, dotted keys, flat names, and"fit.A"-style paths into model fields. Missing fields take exactly the valuesbuild()would give them. Only the header is required:Encodings
"false"for a bool raises.{model = "<qualified registry name>", <every field>...}, recursively. Read back through the field's kind (Kind.get), so aliases, import paths and plugins work. A bare name gives the default model; a table withoutmodelchanges fields of the default model.{value = ..., unit = "solMass / littleh"}(plusdtypeif not float64). Units are parsed withparse_unit(littleh enabled). A unit with littleh goes throughunits.to_canonical, so a wrong littleh power raises the boundary'sUnitConversionError.{value = [...], dtype = "int64"}name, every parameter (m_nuas a Quantity array) andmeta. Loads back equal undercosmology_key. Name and meta are information only and are not hashed.DiskCache, other attrs values{class = "hmf.core.mass_definitions:SOCritical", overdensity = 500.0}. A short name such as"500c"also reads.class = "<import path>"key (e.g. mass definitions,MassVariance.power)CAMB.settings/CLASS.class_paramsNoneNonein the mapping. In TOML it is left out when the field's default isNone, and written as{none = true}otherwise.Non-physics fields are marked with
field(physics=False). They are kept in the config but excluded from the content hash;disk_cacheis the only one so far.Migration and version policy
Schema version.
SCHEMA_VERSION = 1.Migrations. These live in an explicit registry,
MIGRATIONS[n], of functions from schema n to n+1, registered with@register_migration(n, "description").migrate=False, an older schema raises instead.ValueError.Package versions. Versions that differ from the running ones are logged at WARNING; with
strict=Truethey raise aValueErrorinstead.Default changes are schema changes.
test_default_config_is_the_golden_filecomparesMassFunction.build()(without versions) totests/core/data/config_schema_1_default_mass_function.toml. Its failure message says to:That label is added to
.github/labels.yml, with a ":warning: Changes results" category in.github/release-drafter.yml. The policy is documented indocs/core.rstanddocs/developing.rst.Errors. An unknown field raises a
TypeErrorwith "Did you mean". An unknown model raisesModelNotFoundErrorwith suggestions. Each error carries a note saying where in the config it occurred.Content hash; no disk-cache key change
config.content_hash(stage)hashes the configuration's physics plus the versions of hmf, camb, classy and astropy. numpy and scipy are recorded but not hashed. The hash is unchanged by a round trip, bydisk_cache, and by a cosmology's name or meta._serialise.canonicalnow handles plain data itself and delegates every other value toconfig.canonical_value.Boltzmann run keys hash plain run inputs, which are encoded exactly as before. So the disk-cache keys do not change and the key version is not bumped:
test_disk_cache_keys_are_unchangedpins a key.Model content hashes change form (
modelinstead of__class__), but nothing keys on them.Pickling design
Stage.__reduce__returns(config._unpickle, (to_config(stage), runs)), whererunsare the Boltzmann runs the tree has already used. The runs are found by walking fields and computed derived stages, and stored as plain arrays keyed by their run key (_boltzmannnow tracks keys in aWeakKeyDictionary, withexport_runs/import_runs).Unpickling seeds the in-process memo and rebuilds the tree from the configuration, so:
copy.deepcopygoes the same way.Tests cover:
pickletools, has nocamborclassyglobals;clear_memo(), unpickling anddndm;Type of change
Linked issues
Fixes #391
Checklist
I have
tests/core/test_core_config.pyhas 100 tests and 100% line coverage ofconfig.py. It covers:FromArray/TabulatedPower;MassFunctions, with bit-identical results through both the mapping and TOML;caplog), pickling and content hashes;docs/core.rst"Configurations",docs/developing.rst, anddocs/api_core.rst.Local checks run:
tests/core+tests/regression: 2304 passed, 11 skipped;benchmarks/test_gates.py: 25 passed;prekon changed files, and the docs build with-W.Out of scope here:
Reed03andCourtinannotateA: float | None, and Reed03's doc says "None to normalise it". But their converter isfloat, soA=Noneraises. That is pre-existing; the{none = true}path is tested with a local stage instead.Generated by Claude Code
Summary by Sourcery
Introduce a versioned configuration system for reproducible hmf stage serialization, TOML interchange, content hashing, and safe pickle restoration.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: