Add regression tests for stale Zalmoxis mesh on resume - #834
Open
timlichtenberg wants to merge 13 commits into
Open
Add regression tests for stale Zalmoxis mesh on resume#834timlichtenberg wants to merge 13 commits into
timlichtenberg wants to merge 13 commits into
Conversation
Resuming a run at an earlier row while data/zalmoxis_output.dat on disk still holds a later row's mesh crashes Aragog's EOS-radius-range guard, because the file's top radius no longer matches the resumed row's R_int. Add a unit test that pins this with real archived R_int values and discriminates a stale mesh from a freshly regenerated one against validate_zalmoxis_output_schema, and an integration test that drives zalmoxis_solver with real EOS physics on an archived reference run and checks the regenerated mesh the same way. The integration test skips when the archived config carries keys this checkout's schema does not recognise, since that reflects which branch produced the fixture rather than the mesh regeneration under test.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| # Consistent with the row's post-solve state, checked against the file's | ||
| # own recomputed mass integral rather than the solver's in-memory one. | ||
| result = validate_zalmoxis_output_schema(str(regenerated_path), hf_row) |
| fresh_path, r_cmb=r_core, r_surf=resumed_r_int | ||
| ) | ||
| hf_row = {'R_int': resumed_r_int, 'M_int': m_mantle + m_core, 'M_core': m_core} | ||
| result = validate_zalmoxis_output_schema(fresh_path, hf_row) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #834 +/- ##
==========================================
+ Coverage 93.02% 93.16% +0.14%
==========================================
Files 112 112
Lines 16680 16689 +9
Branches 2975 2976 +1
==========================================
+ Hits 15517 15549 +32
+ Misses 1163 1140 -23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The integration test pointed its only fixture at a run directory outside the repository, so it skipped on every machine but the one that produced it, including CI. I trimmed a single row and mesh file from that archived run down to tests/data/integration/zalmoxis_resume_mesh/, stripping the accretion table left over from an in-progress module and backfilling two escape-tracking columns the schema has since added, so the fixture loads and the solve actually runs. I also widened the config-load skip guard from UnknownConfigKeyError to (UnknownConfigKeyError, ValueError), since a schema-incompatible optional module (atmodeller) surfaces as a plain ValueError rather than the narrower exception, and the guard only wraps the config load, not the mesh regeneration this test exercises.
The docstring described the resumed and stale R_int values as an order of magnitude below the planet's physical scale and four orders above the schema check's 1e-6 relative tolerance. The actual relative drift is about 9.7e-4, which is about three orders of magnitude below the planet's own radius and about three orders above the tolerance, not one and four. The pinned numeric values and the assertion itself were already correct; only the prose describing them was wrong.
The skip guard only wrapped the config load, so a future helpfile column addition would raise HelpfileSchemaDriftError from the unguarded ReadHelpfileFromCSV call right after it and fail the test with an error rather than skip, contradicting what the module docstring already promised. I moved that call inside the try block and added HelpfileSchemaDriftError to the caught exceptions, so both schema drift on the config side and on the helpfile side skip cleanly. I also tightened the docstring: it now says plainly that the two backfilled escape columns are not physically consistent with the row's own non-zero escape rate and should not be read from this row for anything escape-related, and that the row still carries two columns left over from an abandoned accretion module that are harmless because ReadHelpfileFromCSV only checks for missing columns, not extras.
… fixture The mesh-resume test's archived config carried its original run's outgassing module, atmodeller, an optional package most checkouts don't install. zalmoxis_solver never reads config.outgas, so the choice has no bearing on what this test checks, but loading the config still validates every table including outgas, so a checkout without atmodeller failed to load the fixture with a schema-drift-looking error that actually meant "package not installed." Switch to calliope, the standard outgassing backend that ships with every checkout, so the fixture loads everywhere this test is meant to run.
The fixture's mantle EOS, PALEOS-2phase:MgSiO3, is a real Zenodo-hosted table that isn't part of every checkout's FWL_DATA cache and isn't among the tables PROTEUS's standard CI setup downloads. zalmoxis_solver sits outside the test's schema-drift skip guard, so on a checkout missing the table the solver raised and the test errored instead of skipping, with a message that gave no hint the real cause was missing data rather than a regression. Add a narrow pre-flight check right before the solver call, using the same EOS-file-presence check zalmoxis_solver itself runs internally, and skip with a specific reason when it reports a missing table. Any other exception from the solver call still propagates as a real test failure. Update the module docstring to note the table's provenance.
…mantle assumption The pre-flight check mirrors zalmoxis_solver's layer_eos_config construction only up to the point where the solver extends the mantle entry with volatile components, a step it applies whenever dry_mantle is false. The fixture is dry_mantle=true today, so the two constructions agree, but a future edit to a wet mantle would make the pre-flight check silently under-check: it would still report the base table present while the solver's own volatile-extended lookup could fail on tables the check never inspected. Assert dry_mantle up front so that mismatch surfaces as a clear test failure telling the next editor to extend the check, rather than as a resurfaced uncaught RuntimeError from inside the solver.
check_zalmoxis_eos_files() required a Seager2007:iron table for the mesh-resume fixture even though its config sets core_eos to a separate flat PALEOS:iron entry that never touches the mantle registry entry's nested 'core' sub-entry. Every runtime lookup that resolves the core layer's EOS does so from the independently-configured core_eos string, never through a mantle or ice_layer entry's embedded 'core' key, so that nested content only needs checking when an identifier is itself filling the core role. The pre-flight check now tracks which layer role each identifier belongs to and skips a nested 'core' sub-entry unless the outer role is core.
The pre-flight check before zalmoxis_solver() only catches a missing table if its own layer_eos_config construction matches what the solver builds internally. zalmoxis_solver resolves the CMB temperature before its own internal EOS check ever runs, so a missing table can still raise an uncaught FileNotFoundError or RuntimeError from inside the solver call itself, bypassing the pre-flight skip entirely. Wrap the solver call in a try/except that skips only on those two specific missing-data exceptions and lets any other exception, including a genuine solver-convergence failure, propagate as a real test failure.
…ption The FileNotFoundError zalmoxis_solver raises for a missing 2-phase mantle table has no path attached: numpy's DataSource.open builds it from a bare message string, and the missing table degrades to an empty file path several calls before that read, so even the message text names no path. Catching FileNotFoundError and re-running the same pre-flight EOS check against the live filesystem gives an accurate skip reason instead, and only skips when the re-check confirms a table is missing; anything else still fails the test as a real regression.
The existing nested-siblings test reuses the same file path for the standalone core entry and the nested 'core' sub-entry it is meant to check is excluded from a mantle-role request, so it passes identically whether or not that exclusion exists. The new test uses a registry where the nested 'core' sub-entry names a missing, distinct path, so a core-role request must flag it while a mantle-role request for the same identifier must not.
check_zalmoxis_eos_files raised a bare RuntimeError, which forced every catch site (production and tests) to either accept any RuntimeError or match on the exact message prefix. Introduce ZalmoxisMissingEOSFilesError so both sides can catch the specific failure instead. load_zalmoxis_configuration resolves the CMB temperature before zalmoxis_solver's own pre-flight check runs, so a missing table could still surface as a bare, unclassified FileNotFoundError from deep inside that resolution step. Wrap that call in try/except FileNotFoundError: on catch, rerun check_zalmoxis_eos_files against the live filesystem and let it raise the dedicated exception; if the recheck finds nothing missing, re-raise the original FileNotFoundError unchanged. The recheck only covers a missing EOS table. A missing melting-curve directory for a WolfBower2018/RTPress100TPa mantle EOS is a different file class that neither this recheck nor zalmoxis_solver's own pre-flight check inspects, and re-raises unconverted from both call sites, for every temperature_mode; that gap is pre-existing and stays open here. Added two unit tests that force each branch of the try/except directly, mock load_zalmoxis_material_dictionaries so they stay hermetic, and assert on the arguments the recheck passes to check_zalmoxis_eos_files. check_zalmoxis_eos_files also dropped a registry entry's nested 'core' sub-entry whenever a non-core role referenced it, which is only correct when the entry has other role-specific sub-entries to use instead (the PALEOS-2phase:MgSiO3-with-core case). A single-key 'core' entry such as Seager2007:iron has nothing else to fall back to and real dispatch reads that sole sub-entry regardless of role, so it must always be checked. Added a test covering that shape from both a mantle and an ice_layer role. Fixed the docstring on resolve_2phase_mgsio3_paths, which claimed a (None, None) pair on failure; the two paths resolve independently and one can be present while the other is None. No caller relies on the old all-or-nothing framing, so only the docstring changes. The integration test no longer hand-copies layer_eos_config or runs its own pre-flight check: it calls zalmoxis_solver directly and catches ZalmoxisMissingEOSFilesError, which now covers a missing table regardless of where in the call chain it is detected.
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
1 participant
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.
Description
This adds regression tests for the resume mesh crash: resuming a run at an earlier row while
data/zalmoxis_output.daton disk still holds a later row's mesh crashes Aragog's EOS-radius-range guard, because the file's top radius no longer matches the resumed row's R_int. The root cause is a gap in the fleet's own replay/test recipe: it truncates a run for replay but never regeneratesdata/zalmoxis_output.datafter truncating the helpfile, so the stale mesh from the run's later state is left on disk. This is not a PROTEUS production bug and no production code is touched; a genuine single-crash resume always restarts from the helpfile's last row and does not hit this path.Validation of changes
Added a unit test in
tests/interior_struct/test_zalmoxis.pythat pins the real archived R_int drift (about 0.097%) between a resumed row and a stale later-row mesh, and confirmsvalidate_zalmoxis_output_schemaaccepts a mesh regenerated at the resumed row's own R_int while rejecting the stale one, well above the check's tolerance. Added an integration test intests/integration/test_integration_zalmoxis_resume_mesh.pythat driveszalmoxis_solverwith real EOS physics against an archived reference run and applies the same discrimination check to the regenerated mesh; on a main-based checkout this currently skips because the archived fixture's config carries keys from an unmerged branch, and passed with real physics in an observed run under a checkout that had those keys (261.31s). The unit test runs on synthetic mesh files and is unaffected by that fixture issue.Test configuration: macOS, Python (conda
proteusenvironment). Ran both new tests directly with pytest; the unit test passes on this checkout, the integration test skips for the reason above.Checklist