Update(ModelTime): refactor ModelTime and add new features #2367
Conversation
* refactor call signature to accept perlen, nstp, and tsmult as parameters * added methods: - `set_start_datetime()`: allows user to set/reset the start_datetime - `set_time_units()`: allows user to set/reset the time_units - `timeunits_from_user_input()`: uses fuzzy string logic to parse user supplied time units - `datetime_from_user_input()`: converts many types of datetime representation to a standard datetime object - `get_totim()`: allows user to get the totim value at the end of a stress period or stress period/time step combination - `get_datetime()`: allows user to get the datetime value at the end of a stress period or stress period/time step combination - `intersect()`: method that allows for time intersection. Method accepts datetime representations or totim and returns either the zero based stress period or the stress period and time step combination * added testing for ModelTime * updated FloPy model classes for ModelTime changes
* add get_datetime_string() method to ModelTime to format ISO 8601 compliant date times
|
will this close #1915? |
|
This will close #1915. The |
There was a problem hiding this comment.
Hey Josh, I had a bunch of little comments, many just about terminology. Just thought I'd bring them up for discussion before they get burned in. I think what you've done here will be widely used.
One thing to consider: we have adaptive time stepping now, which doesn't fit the mold of nstp/tsmult. Not sure how we should handle it, but I think we should leave space for it somehow. Unfortunately, we don't know the time stepping information until after the simulation runs, but we could have ModelTime load the information. We can modify ATS to write time step information, which might be helpful.
Is there a way to get a list of datetimes that correspond to the end of each time step? Seems like this would be great for making plots?
flopy/discretization/modeltime.py
Outdated
|
|
||
| return dt | ||
|
|
||
| def intersect(self, datetime_obj=None, totim=None, kper_kstp=False, forgrive=False): |
There was a problem hiding this comment.
| def intersect(self, datetime_obj=None, totim=None, kper_kstp=False, forgrive=False): | |
| def intersect(self, datetime_obj=None, totim=None, kper_kstp=False, forgive=False): |
Note that forgive is misspelled in a few places.
Also, I wonder if intersect is the best name for this? Would this be better explained with find?
There was a problem hiding this comment.
updated the forgive flag.
We should think about the naming here, because I think that this is going to be one of the more used methods of the class. I'm not sure that find is quite right for the method name
…roperty to get a list of datetimes at the end of each time step
| if modeltime.start_datetime != start_datetime: | ||
| raise AssertionError("start_datetime improperly stored") | ||
|
|
||
| result = modeltime.intersect("3/06/2024 23:59:59") |
There was a problem hiding this comment.
I know intersect is used elsewhere, but consider renaming this method to say what it does, for example get_kstpkper() (if I'm understanding this correctly)
|
I think that's all I have for now. Thanks for putting this together @jlarsen-usgs. Is there a way to see test coverage or is that turned off for draft requests? |
*update test_model_splitter
|
Added |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2367 +/- ##
=========================================
+ Coverage 68.4% 76.3% +7.8%
=========================================
Files 294 293 -1
Lines 59390 59996 +606
=========================================
+ Hits 40652 45798 +5146
+ Misses 18738 14198 -4540
|
manually add new modeltime.py
manually add new version of test_modeltime.py
…osely misspelled unit names
| headings are: perlen, nstp, tsmult | ||
| temporal_reference : TemporalReference | ||
| contains start time and time units information | ||
| perlen : list, np.ndarray |
There was a problem hiding this comment.
Could the type be ArrayLike? As implemented, can a scalar be provided (also for nstp and tsmult) indicating a sim with a single period?
refactor call signature to accept perlen, nstp, and tsmult as parameters
added methods:
set_start_datetime(): allows user to set/reset the start_datetimeset_time_units(): allows user to set/reset the time_unitstimeunits_from_user_input(): uses fuzzy string logic to parse user supplied time unitsdatetime_from_user_input(): converts many types of datetime representation to a standard datetime objectget_totim(): allows user to get the totim value at the end of a stress period or stress period/time step combinationget_datetime(): allows user to get the datetime value at the end of a stress period or stress period/time step combinationintersect(): method that allows for time intersection. Method accepts datetime representations or totim and returns either the zero based stress period or the stress period and time step combinationadded testing for ModelTime
updated FloPy model classes for ModelTime changes