fix(prt): support exchange coupling to ATS flow models by wpbonelli · Pull Request #2753 · MODFLOW-ORG/modflow6

wpbonelli · 2026-04-03T14:17:19Z

Support exchange-coupling PRT to GWF models that use adaptive time stepping (ATS). This would previously produce incorrect results, as PRT advance/solve routines are generally stateful where the other models are idempotent.

Make these routines idempotent by "staging" tracking state during each solve and only "committing" it on successful time steps.

This also corrects PRT's behavior when it's solved in the same solution group with a flow model with mxiter > 1. Flopy puts models in the same solution group by default, but we didn't hit this before now because mxiter = 1 by default.

This required fixes in mem_reallocate() routines to correctly handle the array shrinking case (e.g. particles are released in a time step that then fails, so need to be "unreleased"). Existing uses must have been limited to expanding.

Also required buffering track records in memory during the time step, and reporting them when the model OT hook is called for a successful time step. The buffer size starts at 64, doubles lazily as needed, and never shrinks, just overwritten on retried steps. (The starting size seems like an arbitrary choice; the release point count seemed like a reasonable guess, but there is no guaranteed relationship between that and the number of particles ultimately released, so I just made it a constant).

This is a departure from the pattern up to now, which was to write particle events immediately, a deliberate choice motivated by MP7 suffering OOM errors when consolidating internal scratch files before writing the final output file for large models. But since just one time step is buffered at once the max usage should typically still be lower than MP7.

Checklist of items for pull request

Replaced section above with description of pull request
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.toml 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

aprovost-usgs · 2026-04-22T12:39:43Z

+[[items]]
+section = "fixes"
+subsection = "model"
+description = "A PRT model could not previously be coupled via an exchange to a GWF model that uses adaptive time stepping (ATS). This is now supported."


is the issue that it could not be coupled, or that when coupled to a GWF-ATS model it did not work properly (in that it did not reset the particles)?

Good point, it could be coupled but the results would be incorrect. Will improve the phrasing.

aprovost-usgs · 2026-04-22T13:18:19Z

+      do i = 1, this%ntrackfiles
+        file = this%files(i)
+        if (file%iun > 0 .and. &
+            (file%iprp == -1 .or. file%iprp == rec%iprp)) &


It looks like the ParticleTracks object is in charge of determining where the buffered records are written. To do this, it needs to be aware of the kinds of things that can have an associated output file -- currently, the model (iprp == -1 ?) and the prp's. just wondering whether there's a (reasonable) way to arrange this so that ParticleTracks doesn't need to be aware of that, and whether that would be desirable. I.e., the model and the prp's (and whatever other objects creating track output there might hypothetically be in the future -- advanced packages?) decide for themselves which records they want in their output. Maybe each even has its own buffer? Could be something to chat about at some point.

Nice idea.. Currently the model owns a single ParticleTracksType which knows about model- and PRP-scoped output files, which is awkward. Would make more sense for each to manage its own output. It would be really nice if model and package could select events independently. Need to consider how to configure that, since event selection is in OC.

aprovost-usgs · 2026-04-22T13:29:11Z

    ! members
    type(PrtFmiType), pointer :: fmi => null() !< flow model interface
    type(ParticleStoreType), pointer :: particles => null() !< particle store
+    type(ParticleStoreType), pointer :: particles_old => null() !< old particle state (for ATS)


From the point of view of anything within PRT, does it matter why the tracking is being retried (e.g., because of ATS, or because of Picard)? If not, then maybe most comments (like this one) don't need to refer specifically to ATS, except perhaps in one place just to cite examples of why you might need this retry capability. In other words, maybe the comments can be less ATS-specific overall.

Will remove mention of ATS here. It may not be the only use case for the old state snapshot in the future, in any case.

it does matter why the advance hook is being called though (detail in comment below)

aprovost-usgs · 2026-04-22T13:33:30Z

-    ! Reset state variable
-    irestore = 0
-    if (iFailedStepRetry > 0) irestore = 1
+    ! The model should advance just once per (step, ATS retry) pair.


Mentioning ATS and Picard as specific examples of retries is good here. However, instead of calling it a "(step, ATS retry) pair" up front, you might consider just "(step, retry) pair", and maybe even rename iFailedStepRetry to simply iRetry to keep it generic?

iFailedStepRetry comes from SimVariablesModule, so it'd need a local variable if we want to clean this up (which I'm not against doing)

Re: ATS vs Picard, I tried to clarify the comment. The PR description didn't really nail this either at first, I'll edit that too. I figure it may help to reserve "retry" to mean "ATS retry" and "rerun" or "repeated invocation" or similar for when advance is called more than once. Retry reruns need to run normally, but not Picard-triggered reruns.

aprovost-usgs · 2026-04-22T13:35:04Z

    ! Coincident release times are merged to
    ! a single time by the release scheduler.

+    ! Save or restore particle state for ATS


"for retry"?

aprovost-usgs · 2026-04-22T13:35:58Z


-    ! Note: particle tracking output is handled elsewhere
+    ! Flush buffered track events to disk. prt_ot is called once per
+    ! converged time step from Mf6FinalizeTimestep, outside the ATS


"outside the retry loop"?

maybe just not say anything here, and let it be explained elsewhere?

aprovost-usgs · 2026-04-22T13:37:16Z

+  !! only mutable state (positions, status, tracking time, etc) without
+  !! immutable state (identity, options) saves space and time, but note
+  !! that it assumes this store already has all the immutable state; if
+  !! this routine is ever used beyond the context of ATS compatibility,


"retry (e.g., ATS) compatibility"?

updated phrasing

aprovost-usgs · 2026-04-22T13:38:46Z

+  !> @brief Buffer a particle event for deferred write.
+  !! The buffer can be flushed to disk by flush_buffer,
+  !! or discarded by discard_buffer (e.g., on a failed
+  !! ATS retry). The buffer starts at 64 events, then


this reference to ATS as an example is good i think

aprovost-usgs · 2026-04-22T13:39:18Z

+  end subroutine flush_buffer
+
+  !> @brief Discard buffered events.
+  !! Called from prt_ad at the start of each new ATS retry attempt,


"each new tracking retry"?

aprovost-usgs

I have some comments/questions, but don't let them hold up the merge unless you feel there's more discussion needed

wpbonelli · 2026-04-24T19:15:23Z

#2789 should go before this

wpbonelli added this to the 6.7.1 milestone Apr 3, 2026

wpbonelli added the bug label Apr 3, 2026

wpbonelli force-pushed the prt-ats branch from d00343b to b0993dd Compare April 14, 2026 15:16

wpbonelli marked this pull request as ready for review April 14, 2026 23:52

wpbonelli marked this pull request as draft April 15, 2026 00:30

wpbonelli force-pushed the prt-ats branch 2 times, most recently from d24b5ec to 4278c86 Compare April 16, 2026 02:41

wpbonelli marked this pull request as ready for review April 16, 2026 02:42

wpbonelli requested a review from aprovost-usgs April 16, 2026 02:43

wpbonelli and others added 19 commits April 16, 2026 05:24

fix(prt): support exchange coupling to ATS flow models

b9849e9

ruff

92c52d3

revert prt fix

a51f444

use existing ats model in new test

9741b66

tests

e5a809b

revert prt.f90

c88f7ac

debugging

1f4dc94

fixed

acbbefe

clean up

9c99d2d

fix test

9f36615

release note

e000369

comments

1428093

seek/truncate output files on failed step, fix rerun guard

3e5ab1d

close/reopen necessary for some platforms (linux + gfortran at least)

2c873aa

buffer instead of seek/truncate

fd28f54

dont export unused routine

326ee55

cleanup

2f5e3eb

ruff

56e212b

rename to TrackRecordType

09ec5cc

wpbonelli force-pushed the prt-ats branch from d413c50 to 09ec5cc Compare April 16, 2026 12:25

move guard to prt advance

4780f9f

wpbonelli added 2 commits April 17, 2026 18:06

clean up

86e5516

spelling

93f321c

wpbonelli commented Apr 20, 2026

View reviewed changes

Comment thread src/Solution/ParticleTracker/Particle/ParticleTracks.f90 Outdated

wpbonelli added 3 commits April 20, 2026 10:51

fix docstring

72919a0

better comments

4027bc6

better docstring

da48231

aprovost-usgs reviewed Apr 22, 2026

View reviewed changes

aprovost-usgs approved these changes Apr 22, 2026

View reviewed changes

wpbonelli added 6 commits April 22, 2026 15:57

improve comments

d80a097

more tweaks

4ed1946

one more

64e724c

need to solve prt again on Picard iterations

1df2d2e

optional scratch file buffer

979d4ab

cleaner

b6fed9d

wpbonelli mentioned this pull request Apr 24, 2026

feat(prt): configurable event buffering #2789

Open

5 tasks

wpbonelli marked this pull request as draft April 24, 2026 19:14

Conversation

wpbonelli commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aprovost-usgs left a comment

Choose a reason for hiding this comment

Uh oh!

wpbonelli commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wpbonelli commented Apr 3, 2026 •

edited

Loading