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

Classic Modflow time-related classes and start_datetime #1915

Open
mwtoews opened this issue Aug 11, 2023 · 1 comment
Open

Classic Modflow time-related classes and start_datetime #1915

mwtoews opened this issue Aug 11, 2023 · 1 comment
Assignees
Milestone

Comments

@mwtoews
Copy link
Contributor

mwtoews commented Aug 11, 2023

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 wpbonelli added this to the 3.7.0 milestone May 1, 2024
@wpbonelli
Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants