modflowpy / flopy

A Python package to create, run, and post-process MODFLOW-based models.
https://flopy.readthedocs.io
Other
517 stars 313 forks source link

Change in default output control behavior #276

Closed samzipper closed 6 years ago

samzipper commented 6 years ago

I am new to FloPy (and Python) and am working through the tutorials on the website. For Tutorial 2, I am having an issue in which output is only saved at the first (steady-state) timestep.

In the documentation for the mfoc module, it notes that the default behavior for stress_period_data is to save head at all timesteps:

Dictionary key is a tuple with the zero-based period and step (IPEROC, ITSOC) for each print/save option list. (default is {(0,0):[‘save head’]}) ... The list is used for every stress period and time step after the (IPEROC, ITSOC) tuple until a (IPEROC, ITSOC) tuple is entered with and empty list.

However, it looks like this description is a bit dated, as the release notes for version 3.2.7 state that each timestep where output is desired must be explicitly specified:

OC package: Fixed bug when printing and saving data for select stress periods and timesteps. In previous versions, OC data was repeated until respecified.

For Tutorial 2, I am able to reproduce the imshow head contour plots by manually specifying output at the necessary timesteps:

# Output control
spd = {(0, 0): ['save head','save drawdown'],
       (1, 99): ['save head','save drawdown'],
       (2, 99): ['save head','save drawdown']}
oc = flopy.modflow.ModflowOc(mf, stress_period_data=spd,
                             compact=True)

However, I am not able to reproduce the plot of head vs time because there is only output at the three timesteps I define.

Is there a recommended solution to produce output at all timesteps, without creating a very long dictionary to pass to mfoc? For example, a flag that can be passed to stress_period_data?

Thanks for all your work creating/maintaining this excellent package!

jtwhite79 commented 6 years ago

I tend to agree that heads should be saved at the last time step of every stress period by default. I think the save_head_every option was the easy way to do this. Maybe if stress_period_data is None, then we do this by default? I can work on this if no one has any objections.

langevin-usgs commented 6 years ago

As long as there are no side effects. It has been a real pain to get some simple default behavior for output control. Do we need to rethink some of this? Maybe save_head_every isn't the best argument name since we may want to include budget in this as well?

jtwhite79 commented 6 years ago

I agree that getting good defaults for OC has been a struggle. I wasn't planning to bring back the save_head_every, but I was thinking that is stress_period_data is None, then reset to write heads at the end of each stress period. We also include budget in the default as well?

samzipper commented 6 years ago

Obviously I don't know the backstory here so maybe this has already been considered and rejected, but to the novice user it seems like there is value in a quick way to print all output. In the example above, something like this may be a potential approach that doesn't revive save_head_every:

spd = {(0, :): ['save head'],
       (1, :): ['save head'],
       (2, :): ['save head']}
# OR
spd = {(:,:): ['save head']}

oc = flopy.modflow.ModflowOc(mf, stress_period_data=spd,
                             compact=True)
langevin-usgs commented 6 years ago

I like that a lot.

jtwhite79 commented 6 years ago

I started working on this and (I don't think) we can use (:,:) because the : isn't defined outside of the [] getitem indexer. So what should we use to indicate "all"? My first thoughs wereNone or a Slice instance, but None usually has a different meaning and a slice instance is not-so-easy to construct. But def like the idea of having an "all" indicator. Maybe string "all" would work?

langevin-usgs commented 6 years ago

I guess 'all' is okay. But it something just seems unsatisfying.

jtwhite79 commented 6 years ago

So after some digging, it looks like there is an undocumented save_every optional keyword arg with a fair amount of logic (started around line 184 in mfoc.py). And I also don't like the "all" idea..so I added a new option that if stress_period_data is passed as None, you get heads and budget saved for the last time step of every stress period. I haven't changed the default value of stress_period_data yet, but I think it should set to None since that gives a "good" default behavior.

samzipper commented 6 years ago

Thanks for pointing that out. Just to clarify for future beginners who may stumble across this thread, the following command will save head and print budget for all timesteps:

oc = flopy.modflow.ModflowOc(mf, save_every=True, compact=True)

NB: This will override any stress_period_data input. For example in the following case, head is saved at every timestep but you don't get drawdown:

# Output control
spd = {(0, 0): ['save head','save drawdown'],
       (1, 99): ['save head','save drawdown'],
       (2, 99): ['save head','save drawdown']}
oc = flopy.modflow.ModflowOc(mf, stress_period_data=spd,
                             save_every=True, compact=True)