Skip to content

test(postprocessing): check signed magnitudes in 1-D faceflow cases - #2846

Merged
wpbonelli merged 1 commit into
modflowpy:developfrom
TameTheGame:test-signed-faceflow-magnitudes
Sep 9, 2026
Merged

test(postprocessing): check signed magnitudes in 1-D faceflow cases#2846
wpbonelli merged 1 commit into
modflowpy:developfrom
TameTheGame:test-signed-faceflow-magnitudes

Conversation

@TameTheGame

@TameTheGame TameTheGame commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

The existing one-dimensional get_structured_faceflows cases check whether the active-direction array contains nonzero values. Add an independent physical reference: unit conductivity and cell dimensions with prescribed heads 11, 12 and 13 imply face flows [-1, -1, 0] toward increasing column, row or layer number. Check the absent terminal face is exactly zero.

This adds ten lines and preserves all seven existing parameterizations, model inputs, simulation calls and dependencies. It is a test improvement; no existing implementation defect is claimed.

Validation on base 18dc192cac9ed731dc84f40bc898e0f4d9f87e20:

  • All seven targeted cases passed on Windows with zero skips using Python 3.12.13, NumPy 2.3.5, pytest 8.4.2 and the official MODFLOW 6.8.0 Windows release asset, which reports 6.8.0+8680167.dirty.
  • Observed execution confirmed both new assertions ran for all three one-dimensional cases; active-face outputs were exactly [-1, -1, 0].
  • Ruff 0.16.6 lint and format checks passed for the changed file.

The full FloPy test suite and project-wide release checks were not run locally. This PR is ready for review and has received maintainer approval on commit c08bb9d93db82635bd63eb0a063d2042a53710cd. As of September 9, 2026, eleven of twelve main CI test jobs passed, along with build, lint, MODFLOW integration tests and documentation notebook checks. The remaining job, labeled Ubuntu/Python 3.12, failed in test_get_modflow.py::test_python_api[modflow6-MODFLOW-ORG] because the expected executable destination was empty after the download attempt; all seven modified face-flow cases passed in that same job. The download failure appears separate from this assertion change, but its cause remains unconfirmed and CI is not fully green. See the failed job.

Related PR #2746 adds broader conversion/round-trip coverage and leaves this existing assertion block unchanged at the head inspected. This patch is limited to the existing physical 1-D cases.

AI assistance: OpenAI Codex using GPT-6 Astra assisted with the assessment, patch and local validation.

@TameTheGame
TameTheGame marked this pull request as ready for review September 9, 2026 02:45

@wpbonelli wpbonelli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

reasonable addition, though I admit I'm curious what sequence of events led you to this PR. thanks in any case

@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.6%. Comparing base (556c088) to head (c08bb9d).
⚠️ Report is 229 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2846      +/-   ##
===========================================
+ Coverage     55.5%    73.6%   +18.0%     
===========================================
  Files          644      659      +15     
  Lines       124135   132282    +8147     
===========================================
+ Hits         68947    97402   +28455     
+ Misses       55188    34880   -20308     

see 586 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@TameTheGame

Copy link
Copy Markdown
Contributor Author

reasonable addition, though I admit I'm curious what sequence of events led you to this PR. thanks in any case

Just a new open-source contributor looking to start small. Glad it's a worthwhile addition.

@wpbonelli
wpbonelli merged commit 5e9d989 into modflowpy:develop Sep 9, 2026
39 of 40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants