Repository navigation
fix(com4FlowPy): use dx in 3D min-distance calculation - #1342
Merged
fso42 merged 1 commit intoOct 14, 2026
Merged
Conversation
calcDistMin(calc3D=True) computed the 3D term as sqrt(_dy*_dy + _dy*_dy + _dz*_dz), dropping the x-component entirely. The 3D distance is now sqrt(_dx*_dx + _dy*_dy + _dz*_dz), matching the 2D term directly above it. Adds test_calcDistMin3D regression test (dx=30, dy=40, dz=6: expect 50.3587, the old code produced 56.8859). Closes OpenNHM#1341
ahuber-bfw
approved these changes
Oct 14, 2026
ahuber-bfw
left a comment
Contributor
There was a problem hiding this comment.
1 line bugfix + added test, confirming correctness of bugfixed code.
pytest and standardtests pass.
can be merged without changes
optional: add short inline comment referencing the PR (see comment)
thx for the contribution :)
PaulaSp3
approved these changes
Oct 14, 2026
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.
Closes #1341
Cell.calcDistMin(calc3D=True)inavaframe/com4FlowPy/flowClass.pycomputed the 3D term assqrt(_dy*_dy + _dy*_dy + _dz*_dz)— the x-component was dropped (a copy slip; the 2D term on the line directly above correctly uses_dx*_dx + _dy*_dy). The 3D distance is nowsqrt(_dx*_dx + _dy*_dy + _dz*_dz).Added
test_calcDistMin3Dregression test inavaframe/tests/test_com4FlowPy.py: with dx=30, dy=40, dz=6 the fixed code yields minDistXYZ ≈ 50.3587, while the old code produced ≈ 56.8859, so the test fails on the buggy code and passes on the fix.Verified the fixed computation and the test's bug-discrimination with a standalone script (the full test module needs the repo's heavy geo deps, which CI provides).
PR Checklist
pytestlocally without fails — 24/24 passed