modflowpy / flopy

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

Classic Modflow time-related classes and start_datetime #1915

Open mwtoews opened 1 year ago

mwtoews commented 1 year ago

For classic Modflow models, there appears to be two classes to handle time in some-way.

For example, a model object has "tr" and "modeltime" properties:

import flopy

m = flopy.modflow.Modflow.load("freyberg.nam", model_ws="examples/data/freyberg_multilayer_transient")
m.tr
# <flopy.utils.reference.TemporalReference at 0x1faedd76c80>
m.modeltime
# <flopy.discretization.modeltime.ModelTime at 0x1faedb130a0>

Source code for:

There are at least three start_datetime attributes, some that contradict others:

m.tr.start_datetime  # '1/1/1970'
m.dis.start_datetime  # '1/1/2015'
m.modeltime.start_datetime  # '1/1/2015'

As for setters, there are more inconsistencies:

# this doesn't work
m.modeltime.start_datetime = "2001-11-15"
# AttributeError: can't set attribute 'start_datetime'

# but this does work
m.dis.start_datetime = "2001-11-15"
assert m.dis.start_datetime == m.modeltime.start_datetime == "2001-11-15"

# and this works, but seems to be isolated from other workflows
m.tr.start_datetime = "2008-03-17"

My suggestion is to deprecate the .tr property and TemporalReference class (eventually remove flopy/utils/reference.py). Does start_datetime need to be duplicated for the .dis property? Other ideas?

wpbonelli commented 6 months ago

My suggestion is to deprecate the .tr property and TemporalReference class (eventually remove flopy/utils/reference.py).

This seems reasonable though I'll defer to others with more history on the project. If there is agreement the initial deprecation could be done for 3.7.0 due out later this month.

Does start_datetime need to be duplicated for the .dis property? Other ideas?

Since start time is a member of the mf2005 DIS package it seems reasonable to keep it? Agreed that the setter should work on both modeltime and dis, and ideally setting one should propagate to the other?