Anchor LinearTemp/HalfspaceCoolingTemp at the geometric top of the region - #214
Merged
Merged
Conversation
…gion PR #206 fixed LinearTemp and HalfspaceCoolingTemp assuming that Ttop / Tsurface applies at z = 0 rather than at the top of the polygon. It did so by anchoring at Z[end], the topmost *grid point inside* the region. That is only the region's top when a grid node happens to lie exactly on it: on the grid used by LaMEM.jl's phase-transition test (64 cells over [-1000, 50] km, so no node at z = 0) a box with its top at 0 km got Tsurface at -15.6 km, shifting the whole geotherm down by one cell (210 C -> 0 C at that node, 423 C -> 221 C at 32 km). Z[end] is also only the highest point if the indices happen to be ordered that way, which is not the case for the slab-relative coordinate in add_slab!. Note that add_box! was never affected by the original bug: it already measures Zrot from the box top (Origin defaults to the upper corner). The z = 0 assumption only hit the callers that pass absolute coordinates (add_polygon!, add_layer!, add_plate!, add_sphere!, ...), and the #206 change then broke boxes whenever the top is not on a node. Fix: the two thermal structures take optional `ztop`/`zbot` keywords and callers that know the geometric extent pass it (box, layer, sphere, polygon, plate; slab surface at d = 0, volcano surface at depth = 0). The fallback is extrema(Z) rather than Z[end]. LinearTemp's gradient now spans the geometric thickness (ztop - zbot) instead of the node extent, so Tbot is reached exactly at the bottom face. All other compute_thermal_structure methods accept and ignore the keywords, LinearWeightedTemperature forwards them to its sub-structures. Consequences for the reference values #206 re-baselined: six revert exactly to the pre-#206 numbers (box/slab tops on the region top), four are genuinely new (polygon whose top is at depth, and the LinearTemp cases where the gradient is now defined by the geometric thickness). On the LaMEM.jl test grid the results are identical to v0.7.21, so LaMEM.jl's downstream tests (phase transitions, TM_Subduction_example), which failed against main by 0.1-0.2%, should pass again unchanged. Adds regression tests checking, on grids with and without a node on the region top, that the temperature at a given depth equals the analytic halfspace / linear profile; these fail on main and pass here. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.
PR #206 fixed LinearTemp and HalfspaceCoolingTemp assuming that Ttop / Tsurface applies at z = 0 rather than at the top of the polygon. It did so by anchoring at Z[end], the topmost grid point inside the region. That is only the region's top when a grid node happens to lie exactly on it: on the grid used by LaMEM.jl's phase-transition test (64 cells over [-1000, 50] km, so no node at z = 0) a box with its top at 0 km got Tsurface at -15.6 km, shifting the whole geotherm down by one cell (210 C -> 0 C at that node, 423 C -> 221 C at 32 km). Z[end] is also only the highest point if the indices happen to be ordered that way, which is not the case for the slab-relative coordinate in add_slab!.
Note that add_box! was never affected by the original bug: it already measures Zrot from the box top (Origin defaults to the upper corner). The z = 0 assumption only hit the callers that pass absolute coordinates (add_polygon!, add_layer!, add_plate!, add_sphere!, ...), and the #206 change then broke boxes whenever the top is not on a node.
Fix: the two thermal structures take optional
ztop/zbotkeywords and callers that know the geometric extent pass it (box, layer, sphere, polygon, plate; slab surface at d = 0, volcano surface at depth = 0). The fallback is extrema(Z) rather than Z[end]. LinearTemp's gradient now spans the geometric thickness (ztop - zbot) instead of the node extent, so Tbot is reached exactly at the bottom face. All other compute_thermal_structure methods accept and ignore the keywords, LinearWeightedTemperature forwards them to its sub-structures.Consequences for the reference values #206 re-baselined: six revert exactly to the pre-#206 numbers (box/slab tops on the region top), four are genuinely new (polygon whose top is at depth, and the LinearTemp cases where the gradient is now defined by the geometric thickness). On the LaMEM.jl test grid the results are identical to v0.7.21, so LaMEM.jl's downstream tests (phase transitions, TM_Subduction_example), which failed against main by 0.1-0.2%, should pass again unchanged.
Adds regression tests checking, on grids with and without a node on the region top, that the temperature at a given depth equals the analytic halfspace / linear profile; these fail on main and pass here.
Whats the purpose of this PR?
Describe it in more detail below:
Checklist
[BUGFIX],[ADDITION],[DOC], etc.