Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(prt): record user tracking times for stationary particles #2177

Merged
merged 4 commits into from
Jan 30, 2025

Conversation

wpbonelli
Copy link
Contributor

@wpbonelli wpbonelli commented Jan 30, 2025

Fix #2172 and other problems affecting tracking events, in context of the DRY_TRACKING_METHOD STAY option and more generally.

  1. User-selected times were not reported for stationary particles (the reported issue).
  2. Particles stationary in the final time step did not record a time step ending event.
  3. Particles stationary above the water table in partially saturated cells did not record a time step end event.
  4. Particles stationary above the water table in partially saturated cells in the final time step did not record a termination event.
  5. Particles still active at the end of the simulation did not record a termination event.

1-4 are only relevant/possible with newton on.


Checklist of items for pull request

  • Replaced section above with description of pull request
  • Closed issue PRT package creates incomplete tracking times in output .csv #2172
  • Added new test or modified an existing test
  • Ran ruff on new and modified python scripts in .doc, autotests, doc, distribution, pymake, and utils subdirectories.
  • Formatted new and modified Fortran source files with fprettify
  • Added doxygen comments to new and modified procedures
  • Updated develop.tex with a plain-language description of the bug fix, change, feature; required for changes that may affect users
  • Removed checklist items not relevant to this pull request

Copy link
Contributor

Choose a reason for hiding this comment

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

Since these checks are based on the particle being in a cell that can be wet or dry, would it be more appropriate to put them in a lower-level Method subclass (at the Grid level or below)? Thinking that someday we'll likely have methods for particle tracking through other kinds of models/grids/features, and they might not use these particular cell-based checks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do some refactoring per these considerations in a separate PR.

Copy link
Contributor

@aprovost-usgs aprovost-usgs left a comment

Choose a reason for hiding this comment

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

Just the one comment

@wpbonelli
Copy link
Contributor Author

wpbonelli commented Jan 30, 2025

It's tricky to get particle management and reporting right. Should test more thoroughly and encapsulate state manipulation and transitions where we can in type-bound procedures on the particle, store and methods..

@wpbonelli wpbonelli marked this pull request as ready for review January 30, 2025 21:20
@wpbonelli wpbonelli merged commit 8c104d3 into MODFLOW-ORG:develop Jan 30, 2025
7 checks passed
@wpbonelli wpbonelli deleted the prt branch January 30, 2025 22:53
Copy link
Contributor

@aprovost-usgs aprovost-usgs left a comment

Choose a reason for hiding this comment

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

I'm available to chat today

wpbonelli added a commit that referenced this pull request Jan 31, 2025
Reporting was incorrect for permanently unreleased particles. We want to report them once when they were scheduled for release. #2177 fixed an issue where they were reported every time step, but the solution was incorrect, only reporting them if the release occurred in the first time step.

Also fix reporting for timed-out particles. The new status code 10 introduced by #2177 covers not only the simulation ending but the particle reaching its stop time as well.

Update some comments and notes too.
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.

PRT package creates incomplete tracking times in output .csv
2 participants