Keep the interior on the planet giant impacts grow - #817
Open
timlichtenberg wants to merge 9 commits into
Open
Conversation
A giant impact grows the planet between two interior solves. Two things did not follow it. The JAX right-hand side captures the mesh by value when its factory is installed, and the factory was installed once per process. After an impact the numpy mesh was rebuilt for the new structure while the JAX side kept integrating the old one, so the boundary-flux budget and the state-heat integral stopped describing the same planet. The gap shows up as a step in their ratio at every impact, flat in between, and it disappears if the run is restarted, because a restart rebuilds the factory. Record the geometry the factory was built against and rebuild it when the mesh no longer matches, which for a run without impacts never fires. The core-temperature jump guard also treated the impact as a bad solve. The re-melt moves T_core by thousands of kelvin outside the solver, so the jump is identical at every step size and the retry ladder cannot reduce it; the run died at its first impact after burning six attempts. The guard now stands aside on the step a re-melt fires and keeps its full strength everywhere else, reusing the impact_reset flag the temperature clamps already honour. That flag is cleared before the interior solver runs, so the value is kept for the rest of the step.
The rebuild check sat at solver setup, where the mesh has not yet reached the geometry the step will integrate: the structure update propagates a step later, so the first solve after an impact still ran on the old right-hand side and only the second picked up the new one. Moving the check to immediately before the solve puts it where the mesh is final. Measured on a case whose two impacts land inside the first 2000 yr: the booked-flux to state-heat ratio now reads 0.99999 on every ordinary step either side of both impacts, against 0.688 and 0.425 held indefinitely before. The one step after each impact still reads 0.74 and 0.60, which is the CMB term the per-call integrals over-count there, not this.
The option Z factory is now built more than once: it is rebuilt whenever the mesh moves under it. Its failure path was only ever correct for the first build, where there is nothing installed yet, so reporting a fall back to the finite-difference Jacobian is accurate. On a rebuild the solver still carries the factory built against the geometry the rebuild was meant to replace, and that factory stayed installed and in use while the log said the run had fallen back. The result is the failure this whole path exists to prevent: the solve keeps integrating the planet from before the structure changed, silently. Clear the factory and the recorded geometry key together, so the solve-time check turns the path off rather than running it on a stale mesh, and so a later geometry comparison cannot measure itself against a factory that is no longer there. The new test drives a rebuild to failure on a solver that already has a factory installed, and fails without this change.
The JAX right-hand side was built from a copy of the mesh taken when its factory was installed, so it kept describing the planet as it stood at that moment. A giant impact grows the planet, and Zalmoxis re-solves the structure as the mantle freezes; either one replaces the mesh, and the right-hand side went on integrating the old one while every other consumer saw the new one. Comparing a stored geometry fingerprint against the current one, which is how I first approached this, only closes part of it. With mass coordinates the mesh pins its first and last node to the core and surface radii and solves every interior node from the density profile, so a structure change can leave both of those radii untouched and still move the whole interior, along with the pressure, gravity, area and volume arrays the right-hand side is built from. The fingerprint would report nothing had changed. Read the mesh inside the factory instead, alongside the boundary conditions and the heating arrays, which are already reread there for the same reason. The factory runs once per solve, so the mesh can no longer be older than the step it describes, and the fingerprint and its refresh check are no longer needed. The test asserts the mesh is read once per solve rather than once per install, and that the second solve sees a replaced mesh. It fails if the read is moved back out of the factory.
The factory was installed and then a purely diagnostic log line read the mesh radii, still inside the same try. Anything raising in that line reached the handler, which clears the factory, so a working install would have been torn down and the run quietly moved to the finite-difference Jacobian while reporting an install failure. Read the diagnostic values first and install last, so nothing that can fail runs after the install. Also moves a test marker that was left on a helper rather than on the two tests it belonged to, and adds the case that defeated the earlier approach: a mesh whose cell count and both bounding radii are bit-identical while its interior has moved. Mass coordinates pin the first and last node and solve the rest from the density profile, so that case is what a fingerprint on those three numbers cannot see.
The P-S tables are rebuilt whenever the structure solve reruns, and a giant impact raises their pressure ceiling along with the planet's mass. Aragog kept the copy it loaded at startup, so once the planet outgrew that table the deepest cells were evaluated at its edge instead of at their own pressure. On a probe run growing 0.5 to 4.5 Earth masses the core-mantle boundary reaches 1.67 times the starting table's ceiling, 60 percent of the rows sit beyond it, and density there is understated by 15 percent. Two things kept the stale copy in place. The loader's cache key was a list of file names and sizes, and a regenerated table keeps the same grid shape, so every file keeps its length and the key could not tell the new tables from the old. The key now uses the marker the generator already writes beside the tables, which records the pressure ceiling, the grid shape and the material. Second, the tables were read once when the solver was built; they are now read where the mesh is read, once per solve, so both follow the planet the step is about. This closes the reload the structure-update path documented as missing for composition changes. The remaining piece there is unchanged: the entropy carried over from the previous step is still not bounds-checked against a regenerated range, which a composition change can move and a pressure-ceiling change cannot.
The interior solver kept whichever P-S tables it was built with at startup. Those tables are rewritten whenever the structure solve reruns, with a pressure ceiling that grows with the planet, so a run that outgrows its starting table went on integrating the old one. The right-hand side already follows the regenerated tables; this is the same reload for the state-heat integral, which is the other half of the energy budget and the quantity the conservation residual is measured against. That makes it a correctness fix for the diagnostic rather than for the trajectory: on a planet that stays inside its starting table nothing changes, and on one that outgrows it the reported budget was being taken against tables that no longer describe the planet. The reload sits immediately before the solve, so the integral a step books is taken against the tables that step runs on, and the loader is cached on the table parameters so an unchanged table costs one small read.
The compression-work diagnostic inside solver.reset() evaluates the re-read mesh pressures against whatever table object the solver holds, so on the step after an impact regenerates the tables it was evaluated against the outgrown set, which clamps at the old ceiling. The refresh now runs before reset(), and the later refresh inside the retry ladder stays as a cheap cache hit. Two pieces of wiring had no test: the ladder's refresh call (every ladder test ran with no solver wired, so removing the call changed nothing) and the translation of the one-shot impact flag into the per-step flag inside run_interior. Both are now pinned, the first with an ordering assertion that the refresh lands before the first solve, the second across two steps so one impact cannot exempt two. The docstring on the refresh helper now names the consumers that actually read the live table object (the RHS, the per-call energy integrals, and the compression diagnostic) rather than the impact heat booking, which deliberately runs on the pre-impact tables. A wrapper comment claimed a raised pressure ceiling cannot move the entropy range of the regenerated tables; the range is scanned up to the ceiling, so it can, and the comment now says so. Four structural tests carried the physics-invariant marker without asserting an invariant and lose it, and the test file docstring now covers the contract areas the file grew.
timlichtenberg
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## tl/accretion-morrigan-coupling #817 +/- ##
==================================================================
+ Coverage 93.26% 93.31% +0.04%
==================================================================
Files 118 118
Lines 17273 17297 +24
Branches 3167 3172 +5
==================================================================
+ Hits 16110 16140 +30
+ Misses 1143 1136 -7
- Partials 20 21 +1
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:
|
This was referenced Aug 8, 2026
The step that lands exactly on a scheduled giant impact is clamped only to whatever simulated time remains before it. After a long quiescent phase has let dt coarsen, that remaining time can itself be large, so the step absorbing the impact's melt-fraction jump inherits the same coarseness the run had just grown into. campaign-2 hit this on probe18_capsoff_guardtip: an impact landed while dt had coarsened to 1.06e6 yr, the remaining-time clamp only cut it to 2.14e5 yr, and CVODE burned two failed attempts before succeeding at 4.27e4 yr. I added dt.impact_maximum, an optional absolute ceiling on the landing step, applied before the existing minimum-step floor so a misconfigured ceiling smaller than the floor still can't collapse dt to zero near an impact. It defaults to 0 (disabled), so no existing run's behaviour changes unless I set it. Reproduced the coarse-phase landing cheaply with the dummy interior and the analytical accretion module: a proportional-dt run reaching an impact after dt had grown to ~8e5 yr lands on it at ~3.35e5 yr uncapped, and at the configured ceiling once impact_maximum is set. Added unit tests for the new ceiling to test_timestep.py, including that it never beats the minimum-step floor, and confirmed each one fails against the unfixed clamp before passing against this change. I have not reproduced the CVODE retry-ladder itself against the real Aragog solver; the tests above verify the dt-sizing mechanism a coarse-phase impact triggers, not the solver's response to it.
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
When the planet grows by a giant impact, the structure module regenerates the interior mesh and the P-S EOS tables for the heavier planet, but the Aragog wrapper kept solving on the mesh and tables it loaded at init. This stack layer makes the interior follow the planet: the mesh is re-read on every solve, the EOS tables are reloaded when the structure module regenerates them, and the CVODE/JAX solver is pointed at the current tables before each solve. The JAX Jacobian factory is installed last so a failed diagnostic cannot undo it, is validated against the mesh just before the solve, and is cleared when a rebuild fails so a stale factory can never serve the next solve.
Stacked on #789 (the accretion module and Morrigan coupling); this layer contains only the interior-side fixes that accreting planets need.
Known and deliberate: reloading the tables means the carried entropy state is reinterpreted under the new table at each impact, which can move T_core across its sanity guard on large impacts. That is the instrument reading the new planet correctly rather than a defect in this layer; the underlying solidus-crossing behaviour is tracked in FormingWorlds/aragog#26.
Validation of changes
Verified in an accretion run to 4.5 M_earth: at the fourth impact P_cmb reaches 459.6 GPa, 1.67x the 275 GPa ceiling of the tables the run started with, and the reloaded table ceiling tracks to 875 GPa, so the solver stays on tables that contain the planet. Before the fix the run's tables never changed, so the interior was silently evaluated outside its EOS domain from the second impact on.
This layer does not close the post-impact energy budget; the residual across an impact is set by the solidus-crossing behaviour tracked in FormingWorlds/aragog#26, and runs with these fixes measure it honestly instead of on stale tables.
Unit tier:
pytest tests/interior_energetics/ -m "unit and not slow and not integration"green (181 passed) on macOS with Python 3.12. The reload wiring is pinned by ordering tests that fail when the refresh call or the impact-flag translation is removed.Checklist