Raise C line coverage past the 90 percent ecosystem target - #14
Merged
Conversation
The initial partial-pressure solver forced a matrix-free Jacobian onto its trust-region Newton method, but PETSc 3.19's trust-region step needs a matrix that supports transpose products, which the matrix-free approximation does not provide. Every run using IC_ATMOSPHERE 1 (abundances) or 4 (ocean moles) therefore aborted with "Matrix type mffd does not have a multiply transpose defined" before taking a step. The system is tiny, one unknown per volatile plus one per reaction, so build an explicit finite-difference Jacobian instead; it supports the transpose product and costs a handful of residual evaluations.
The suite exercised only the default blackbody and reaction configurations, leaving the option-dispatched physics dark: radiogenic and tidal heating, the steady-state initial condition, constant-entropy and prescribed-core-flux boundaries, the abundance and ocean-moles atmosphere starts, the IVTANTHERMO and ammonia reaction constructors, and the post-step rollback event. Line coverage of the C sources rises from 81 to 91 percent. New cached smoke runs pin each path against an analytical anchor: two-isotope radiogenic heating against the exponential decay law, prescribed and file-based tidal heating against the option value and a per-node ramp, the steady-state start against a node-independent energy flow and the grey-body surface balance, the entropy boundary against the exact option value, the core-flux boundary against the core-cooling baseline, the abundance start against elemental hydrogen and carbon conservation through the equilibrium solve (with oxygen exchanged through the melt fO2 buffer, which the tests require rather than forbid), the ocean-moles start against the OCEAN_MOLES constant, and a four-reaction, seven-volatile configuration against elemental conservation. The rollback test verifies that a surface-temperature event ends the run at the rolled-back state, and that the post-step check refuses to run without rollback support. The new anchors are recorded on the validation pages for energy.c, bc.c, and ic.c. Coverage counters and reports from instrumented builds are ignored.
…unts A C test executable exercises the DimensionalisableField wrapper (scale, unscale, duplicate, local vector, scaling query) with exact round-trip pins, and the equation-of-state tests pin the compositional viscosity prefactor against all four Mg/Si branches of Spaargaren et al. (2020) plus the activation-volume and activation-energy behavior, including the zero-pressure surface limit. Fast-tier line coverage reaches 91 percent locally and the full suite 92 percent, so the fast gate rises to 88 and the full gate to the 90 percent ecosystem ceiling. The badge generator now also publishes per-tier counts (tests-unit, tests-smoke, tests-integration) alongside the total and the CI-split fast and nightly files, matching the badge scheme of the other ecosystem modules; the publish workflow validates all six files and the testing explainer shows the per-tier badges.
There was a problem hiding this comment.
Pull request overview
This PR increases SPIDER’s C line coverage by adding tests that exercise previously untested option-dispatched physics/solver paths, then raises the CI coverage thresholds accordingly and expands published test-count badges.
Changes:
- Adds new smoke/unit tests covering radiogenic + tidal heating, steady-state IC, rollback/poststep behavior, reaction-library conservation, EOS viscosity branches, and the DimensionalisableField wrapper.
- Updates coverage gates (fast/full) in
pyproject.tomland expands badge generation + publishing to include per-tier counts. - Adjusts the atmosphere IC partial-pressure solver configuration in
ic.cto avoid PETSc trust-region incompatibility with matrix-free Jacobians.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/generate_test_badges.py | Adds per-tier badge outputs (unit/smoke/integration) alongside total/fast/nightly. |
| .github/workflows/publish-test-badges.yml | Validates the expanded set of badge JSON artifacts. |
| docs/Explanations/testing.md | Updates displayed badges to the new per-tier scheme. |
| pyproject.toml | Raises fast/full C coverage gates. |
| .gitignore | Ignores gcov/gcovr coverage artifacts. |
| Makefile | Builds the new C unit test executable. |
| tests/c/test_dimensionalisablefield.c | New C-side evaluator for DimensionalisableField wrapper behavior. |
| tests/test_dimensionalisablefield.py | New unit-tier pytest wrapper asserting DimensionalisableField invariants. |
| ic.c | Switches the initial partial-pressure SNES configuration away from matrix-free Jacobian. |
| tests/test_ic.py | Adds smoke-tier IC coverage for abundance- and ocean-moles-based atmosphere starts. |
| docs/Validation/ic.md | Records new IC validation anchors. |
| tests/test_energy.py | Adds smoke-tier coverage for radiogenic decay, tidal heating (scalar + file), and steady-state IC. |
| docs/Validation/energy.md | Records new energy validation anchors. |
| tests/test_bc.py | Adds smoke-tier coverage for prescribed core flux and surface-entropy IC pin. |
| docs/Validation/bc.md | Records new BC/IC-related validation anchor (wording needs alignment with what’s tested). |
| tests/test_reaction.py | Adds smoke-tier test for the full named reaction library configuration and conservation checks. |
| tests/opts/reaction_library.opts | New full-reaction-library options file used by the new reaction smoke test. |
| tests/test_poststep.py | New smoke-tier poststep/rollback tests (needs robustness tweaks vs rounded output filenames). |
Comment on lines
+19
to
+21
| # blackbody50 cools by hundreds of kelvin in the first 100-year macro | ||
| # step, so a 1 K ceiling guarantees the event fires on step one. | ||
| TSURF_CEILING = 1.0 # K |
Comment on lines
+56
to
+60
| # The final output falls inside the first nominal macro step, far | ||
| # short of the 600-year end of an uninterrupted run. | ||
| assert 0 < outputs[-1] < NOMINAL_END | ||
| assert outputs[-1] < 100 # years, within macro step one | ||
|
|
Comment on lines
+148
to
+153
| """One-step run with constant-entropy boundaries and a steady IC. | ||
|
|
||
| The prescribed boundary entropies (2550 and 2650 J/kg/K) bracket | ||
| the 2600 J/kg/K adiabat; the steady-state energy solve keeps the | ||
| perturbed initial condition integrable within one macro step. | ||
| """ |
Comment on lines
+15
to
+18
| **Reference-pinned test**: `tests/test_bc.py::test_constant_entropy_bc_pins_the_initial_surface` | ||
|
|
||
| **Anchor**: Analytical identity: the constant-entropy boundary writes ic_surface_entropy (2550 J/kg/K) onto the surface basic node, preserved through the steady-state energy solve into the initial output. | ||
|
|
The abundance and ocean-moles configurations targeted inventories an order of magnitude away from the equilibrium implied by the configured guess pressures. The Newton solve covered that distance on macOS but diverged on the Linux runners, where a different BLAS walks a different iteration path. The targets now sit 20 to 40 percent off the three-ocean equilibrium of the reaction configuration, and the library configuration carries explicit guess pressures near its solution, so the solve converges from the same starting point on every platform. The oxygen-exchange and water-redistribution thresholds are recalibrated to the measured values at the new targets, keeping twentyfold margins. The rollback-refusal test no longer pins the exact exit code: the unsupported-configuration error surfaces directly on macOS but through the time-stepper wrapper on Linux, so the codes differ while the refusal is the same.
… to 90 The abundance and ocean-moles tests read only the initial condition, so their runs now use zero macro steps and skip the CVODE integration entirely, cutting several minutes from the pull-request tier, which had approached its ten-minute cap. Coverage is unchanged: the stepping paths those runs used to touch are exercised by the other reaction configurations. The fast coverage gate rises from 88 to the 90 percent ecosystem ceiling, justified by the 91 percent the Linux runners measured on this branch.
The refusal test for the post-step check now asserts the documented error message instead of accepting any nonzero exit; the run harness includes the PETSc error lines in its failure message, which previously fell outside the reported stderr tail. The initial-pressure solver's positivity guard now tests the converged unknown itself rather than the value left behind by the last residual evaluation. A new test cross-checks the solved partial pressures of the abundance and ocean-moles routes against each other, validating the converged equilibrium state rather than only the input bookkeeping. The reaction library configuration loses four dead pressure lines that were silently overridden by the guess block further down. The badge generator's integration count now covers exactly the integration tier so its label is accurate. The heating test pins dtmacro explicitly so its decay anchor cannot be broken by edits to the shared options file, and the field-wrapper test exercises the already-scaled no-op guard. Documentation accuracy: the coverage-gate comment describes the ceiling values and their measured margins, the ic.c validation page states the real discrimination margins, the T-cubed guard annotation carries the correct factor, the oxygen-drift docstring attributes the fO2 exchange to the three reactions that carry it, and the Spaargaren anchor gains hand-derived literal pins plus its validation-page entry.
timlichtenberg
force-pushed
the
tl/coverage-to-90
branch
from
July 12, 2026 20:03
c737932 to
e0a3437
Compare
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
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.
What this does
Brings SPIDER's C line coverage from 81 to 91 percent on the fast (pull request) tier and 92 percent on the full suite, measured locally, by testing the optional physics paths the default configurations never enabled. Raises the fast coverage gate to 88 and the full gate to the 90 percent ecosystem ceiling, and publishes per-tier test-count badges (unit, smoke, integration) matching the other ecosystem modules.
Why
The suite exercised only the default blackbody and reaction configurations, so the option-dispatched physics was dark: radiogenic and tidal heating, the steady-state initial condition, the constant-entropy and prescribed-core-flux boundaries, the abundance and ocean-moles atmosphere starts, four of the six named reaction constructors, the post-step rollback event, the compositional and activation viscosity laws, and the field-scaling wrapper. Several of those paths turned out to be broken or untestable until a solver bug was fixed.
Changes
Testing
Open points