Document that jump_points_from_rupture_tree min_depth is in kilometres - #107
Merged
Conversation
Every other distance in this module and in sources.Point/sources.Plane is metres and says so in the attribute name (bottom_m, top_m, width_m). A caller following that convention and passing metres gets no error: the min() clamp silently reduces the value to the fault bottom, so min_depth=5000 is treated as "as deep as possible". sources.closest_points_beneath already documents the unit; the public wrapper just did not repeat it. Fixes #94 Co-Authored-By: Claude Opus 5 <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
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.
Fixes #94
Problem
min_depthwas documented without a unit:It is kilometres —
:530dividessource.bottom_mby 1000 before comparing, andsources.closest_points_beneathmultiplies by_KM_TO_Mon entry.Every other distance in this module and in
sources.Point/sources.Planeis metres, and says so in the attribute name (bottom_m,top_m,width_m,length_m). A caller who follows the surrounding convention and passes metres gets no error: themin(min_depth, 0.99 * bottom_m / 1000, ...)clamp silently reduces the value to the fault bottom, somin_depth=5000is treated as "as deep as possible" and the jump points are computed at the fault bottom instead of at 5 km.sources.closest_points_beneathalready gets this right (sources.py:1682-1683, "in kilometres"); the public wrapper just did not repeat it.Changes
Docstring only, matching the wording already used in
sources.closest_points_beneath.Not addressed here
Whether a
min_depthfar below every source should raise rather than clamp is the thing that makes the unit error silent, but changing it is a behaviour change — left on the issue for a maintainer decision rather than bundled into a docs fix.Verification
pytest tests/test_rupture_propagation.py— 26 passed.ruff check,ruff format,numpydoc lintclean.🤖 Generated with Claude Code