modflow6 icon indicating copy to clipboard operation
modflow6 copied to clipboard

Should UTL-ATS fill stress period data forward?

Open wpbonelli opened this issue 1 month ago • 7 comments

Unlike other packages UTL-ATS does not "forward fill" stress period data (apply a given period config to any subsequent periods unless/until another period block is explicitly specified). Is there a reason for this specific to ATS? Or maybe to utility packages (however SPC does seem to fill forward from a very quick look). If not, should ATS be modified for consistency with other packages in this regard?

This was noticed originally in the context of Deltares imod-python development. As a workaround, imod-python implements forward filling for ATS.

wpbonelli avatar Nov 01 '25 09:11 wpbonelli

For UTL-ATS, the PERIODDATA block is not a PERIOD block, like the ones specified for transient packages. It is analogous to the PERIODDATA block for TDIS. Values for the entire simulation are all in one PERIODDATA block, and I believe the entire block is loaded at once.

christianlangevin avatar Nov 03 '25 02:11 christianlangevin

Ah, understood. Thanks @christianlangevin

wpbonelli avatar Nov 03 '25 11:11 wpbonelli

Thanks for raising this issue, @wpbonelli ! And thanks @christianlangevin for the quick response.

Context: While implementing support for UTL-ATS in iMOD Python, I went in blind and I assumed UTL-ATS would behave like the transient packages. The nasty thing about it was, that I only found out the errors of my ways when I inspected the list file, and looked at the amount of steps in a stress period that I didn't specify in the UTL-ATS.

The current behavior can especially be annoying in the following use-case (that my team faces regularly). Say you have a complex and slow simulation which takes a few hours to run. Stress period 4 to 6 are difficult to solve, for example because of some rapid changes in boundary conditions, and require specifying UTL-ATS. Users specify ATS-UTL data for period 4, thinking this data will be forward filled to stress periods 5 and 6. They start their simulation, and after a few hours they see their model still doesn't converge, this time in stress period 5, which is frustrating. It consequently will take them some diagnosing the list file before they notice that the amount of timesteps was not affected by the data they set for stress period 4 in UTL-ATS.

We therefore added forward filling for UTL-ATS in imod-python. I get why all periods need to be specified in TDIS, hence PERIODDATA, but, from a user-perspective, this shouldn't be necessary for UTL-ATS I reckon. Or am I missing some use-cases here that warrant a need for forcing users to explicitly specify ATS data for every stress period?

If I understand @christianlangevin 's response correctly, the decision for the PERIODDATA block instead of PERIOD in UTL-ATS is mostly technical? I guess because you need to load all UTL-ATS values at once in memory? Or is it for consistency with other utility packages that require explict info for each stress period as well?

I see three potential solutions:

  1. Leave the current behavior as is. Users might run into the same silent problem that I ran into.
  2. Forward fill data. This might be nasty for users who purely want to specify UTL-ATS data just for one stress period (e.g. stress period 1 for a model with poorly defined initial conditions) and expect the current behavior.
  3. Throw error in case stress period is not defined in UTL-ATS. UTL-ATS has a PERIODDATA block and needs to to be consistent with TDIS. The benefit of this is that the intensions of PERIODDATA are enforced. But you lose the benefits of the current behavior.

There is no perfect solution here that will make everyone happy, I guess. What do you think of the 2 alternatives options? And would it be a lot of work to adapt the code?

JoerivanEngelen avatar Nov 03 '25 11:11 JoerivanEngelen

Whoops, I'll reopen while discussion continues. Sorry @JoerivanEngelen

wpbonelli avatar Nov 03 '25 11:11 wpbonelli

I can see your point, @JoerivanEngelen. That would be frustrating. I guess at this point, it could be problematic to change the existing behavior. We can certainly clarify the behavior in the documentation, and I think that would help. Is there any type of warning that we could issue that would help in your case?

BTW, the forward-fill concept could have also worked for TDIS.

christianlangevin avatar Nov 03 '25 13:11 christianlangevin

Some clarification in the documentation would certainly help.

I guess you could throw a warning if not all stress periods have data in the PERIODDATA block. Something like: "WARNING: Stress periods ... in UTL-ATS have no data assigned to them. Default settings will be applied to these periods." You could generalize that to all packages with a PERIODDATA block, I guess (e.g. TDIS).

I see the MODFLOW 6 team wants to maintain a very stable API for its input files, which is laudable and is a strong point of the software. Updating to a new MODFLOW 6 version rarely results in trouble. I reckon keeping things stable, however, is burdening the dev team with extra maintenance supporting outdated features as well. And might furthermore hold back potential improvements to ease-of-use.

I find it interesting to know when the dev team diverts from this and decides to make a breaking change. And how this is communicated to users before and after implementation (release notes I reckon, but semantic versioning as well?).

Wild idea to discuss changes in the API with users: You could utilize the poll in the Github discussion board to discuss propositions for changes in behavior with users (though I'm certain quite some users will miss this). Something like MODFLOW Enhancement Proposals (MEP's). I'm happy to convert this issue to a proposal to discuss as a first test case to see if we can get other users involved in discussing this as well.

If there already is such a thing via other channels that I missed, let me know. And I also understand if the dev team is not so interested in this: More discussion means less hands-on work (coding) done, usually. So shoot!

JoerivanEngelen avatar Nov 04 '25 10:11 JoerivanEngelen

MEPs seem a good idea. :)

Backwards compatibility is very important. But improvements of existing and new features may make this very difficult or awkward. According to the concept of semantic versioning backwards compatibility should only be broken with MODFLOW 7.

Some ideas:

  • Add a switch, that is off by default, to change behavior.
  • Make changes very explicit in release notes and documentation.
  • Provide lint-like tools that can look for difference and potential problems in input files when used with different MODFLOW 6 versions.

In short, make the changes and their implications explicitly known to the user.

pya avatar Nov 06 '25 09:11 pya