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

Support time-series analysis over a subset of the output data #45

Closed
xylar opened this issue Nov 4, 2016 · 6 comments
Closed

Support time-series analysis over a subset of the output data #45

xylar opened this issue Nov 4, 2016 · 6 comments
Assignees

Comments

@xylar
Copy link
Collaborator

xylar commented Nov 4, 2016

Currently, the the code will only work as expected if config option timeseries_yr1 = 1 and timeseries_yr2 is set to something beyond the end of the current run (e.g. 9999), which are the default settings. If timeseries_yr1 > 1, the code crashes, likely because mpas_xarray is expecting the time series to start from year 1 (it isn't currently making use of timeseries_yr1). If timeseries_yr1 = 1 and timeseries_yr2 is set to be less than the total duration of the run, the timeseries analysis may be performed beyond the desired end time. This is because we add one file with date stamp beyond the final date to be sure we're not missing any important output.

The time axis of the xarray data sets should be sliced to be consistent with these two new config options in each timeseries analysis function.

@xylar xylar self-assigned this Nov 4, 2016
@xylar xylar changed the title Support general timeseries_yr1 and timeseries_yr2 Support time-series analysis over a subset of the output data Nov 4, 2016
@pwolfram
Copy link
Contributor

pwolfram commented Nov 5, 2016

@milenaveneziani and @xylar, we will have some other potential mpas_xarray changes too. Is this something we need to fix now or can we wait until it is time to make the other mpas_xarray changes?

@pwolfram
Copy link
Contributor

pwolfram commented Nov 5, 2016

To be clear, this would be for the item:

@xylar
Copy link
Collaborator Author

xylar commented Nov 5, 2016

@pwolfram, it's not clear to me whether the problem here is that mpas_xarray needs to be updated or just that the calls to mpas_xarray need to be updated. I suspect the latter, in which case this fix is independent of other changes to mpas_xarray.

@xylar
Copy link
Collaborator Author

xylar commented Nov 5, 2016

So I think most (perhaps all?) of what is needed to address this issue is to change lines like the following:
https://github.com/MPAS-Dev/MPAS-Analysis/blob/develop/mpas_analysis/ocean/ohc_timeseries.py#L90

    time_start = datetime.datetime(yr_offset+1,1,1)
    time_end = datetime.datetime(yr_offset+1,12,31)
    ds_yr1 = ds.sel(Time=slice(time_start,time_end))

to look more like the corresponding lines in the climatology computations, e.g.:
https://github.com/MPAS-Dev/MPAS-Analysis/blob/develop/mpas_analysis/ocean/ocean_modelvsobs.py#L129

    time_start = datetime.datetime(yr_offset+climo_yr1, 1, 1)
    time_end = datetime.datetime(yr_offset+climo_yr2, 12, 31)
    ds_tslice = ds.sel(Time=slice(time_start, time_end))

@milenaveneziani
Copy link
Collaborator

This was addressed by #48, right?

@pwolfram
Copy link
Contributor

Please reopen if this is a premature closing @xylar.

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

3 participants