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

Change the way time is added to a model #58

Merged
merged 6 commits into from
May 11, 2022
Merged

Change the way time is added to a model #58

merged 6 commits into from
May 11, 2022

Conversation

rubencalje
Copy link
Collaborator

@rubencalje rubencalje commented May 4, 2022

This pull request changes the way time is added to a model:

time = pd.date_range('2000', '2000-1-4')
model_ds = nlmod.mdims.set_model_ds_time(model_ds, time=time)
<xarray.Dataset>
Dimensions:  (time: 4)
Coordinates:
  * time     (time) datetime64[ns] 2000-01-01 2000-01-02 2000-01-03 2000-01-04
Data variables:
    *empty*
Attributes: (12/13)
    time_units:                DAYS
    start_time:                1990-01-01 00:00:00
    nstp:                      1
    tsmult:                    1.0
    steady_start:              1
    steady_state:              0

When steady_start is True, a steady state stress-period is added before the first value of time with a stress period length of steady_start_perlen. The current ways to specify time (transient_timesteps or perlen) are still supported.

The variable time now always contains the time at the end of each stress period (not sure if this was the case before), so heads and other output can be easily added to the model_ds. To minimise double information, nper and perlen are removed from the attributes of model_ds, and are calculated when needed.

@rubencalje rubencalje marked this pull request as ready for review May 10, 2022 07:44
@rubencalje rubencalje requested a review from OnnoEbbens May 10, 2022 07:45
@OnnoEbbens
Copy link
Collaborator

Nice improvements!

I did add something else when I went through the changes. I moved the attributes with time information ('time_units', 'start_time', 'nstp', 'tsmult', 'steady_start' and 'steady_state') from the model_ds to the time DataArray. So instead of model_ds.attrs['time_units'] = time_units you get model_ds.time.attrs['time_units'] = time_units.

I tested locally and it worked. So when the test pass this can be merged.

@OnnoEbbens OnnoEbbens merged commit a5bde90 into dev May 11, 2022
@rubencalje
Copy link
Collaborator Author

Nice improvements!

I did add something else when I went through the changes. I moved the attributes with time information ('time_units', 'start_time', 'nstp', 'tsmult', 'steady_start' and 'steady_state') from the model_ds to the time DataArray. So instead of model_ds.attrs['time_units'] = time_units you get model_ds.time.attrs['time_units'] = time_units.

I tested locally and it worked. So when the test pass this can be merged.

Good change. Maybe we can then also rename 'time_units' to 'units' and 'start_time' to 'start'?

@OnnoEbbens
Copy link
Collaborator

Good one, added this in 8e54b1b

@OnnoEbbens
Copy link
Collaborator

Actually it was not that easy. The 'units' attribute of the time coordinate seams to be locked and I get this error when I try to write it to a netcdf file: ValueError: failed to prevent overwriting existing key units in attrs on variable 'time'. This is probably an encoding field used by xarray to describe how a variable is serialized. To proceed, remove this key from the variable's attributes manually. It is also mentioned here: pydata/xarray#3739. I don't have time now to dive into this. so I will revert the commit for now and create an issue for this.

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

Successfully merging this pull request may close these issues.

2 participants