Skip to content

fix(com4FlowPy): use dx in 3D min-distance calculation - #1342

Merged
fso42 merged 1 commit into
OpenNHM:masterfrom
iamcipherdev:fix/com4flow-3d-distance-typo-2026-10-14
Oct 14, 2026
Merged

fso42 merged 1 commit into
OpenNHM:masterfrom
iamcipherdev:fix/com4flow-3d-distance-typo-2026-10-14

Conversation

@iamcipherdev

@iamcipherdev iamcipherdev commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Closes #1341

Cell.calcDistMin(calc3D=True) in avaframe/com4FlowPy/flowClass.py computed the 3D term as sqrt(_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 now sqrt(_dx*_dx + _dy*_dy + _dz*_dz).

Added test_calcDistMin3D regression test in avaframe/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

  • I ran pytest locally without fails — 24/24 passed
  • I added/updated tests where needed - 1 additional test added
  • I updated documentation where needed - not required
  • Commits squashed/rebased onto latest master — already single commit
  • Standardtest run without difference — numba and python engine tests for com4FlowPy run without diffs

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
@fso42
fso42 requested review from PaulaSp3 and ahuber-bfw October 14, 2026 09:17
@fso42 fso42 added the bugfix label Oct 14, 2026

@ahuber-bfw ahuber-bfw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Comment thread avaframe/com4FlowPy/flowClass.py
@fso42 fso42 self-assigned this Oct 14, 2026
@fso42
fso42 merged commit 40a15f5 into OpenNHM:master Oct 14, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[com4] Typo in 3D distance calculation in com4FlowPy/flowClass.py

4 participants