ESCOMP / CDEPS

Community Data Models for Earth Prediction Systems
https://escomp.github.io/CDEPS/versions/master/html/index.html
20 stars 45 forks source link

Allow "limit" taxmode in streams to use data before or after the years running over if available #101

Open ekluzek opened 3 years ago

ekluzek commented 3 years ago

I've found that the "limit" option for taxmode can be a really good way to find simulation setups that are invalid and will produce bad results. Sometimes the "extend" option is used where it shouldn't as the first or last value in a daily (or even yearly) varying series is a horrible thing to do. As well as the "cycle" option will go back to the beginning year of a transient period when it reaches the end, which is another horrible thing to do.

The problem with limit as it's currently implemented (both in MCT and NUOPC) is that it only considers data within the years that you are running over. So it won't use data outside that range even if it's available in the time-series. In many cases with our datasets we added "extra" data before or after to eliminate this kind of problem, but that data is essentially unavailable because the algorithm doesn't allow it to be used. I think it should only use the one time-sample before and the one time-sample after the years you are running for, but if that data is there it should use it.

I found this problem in a CTSM issue:

https://github.com/ESCOMP/CTSM/issues/1295

Here's the most relevant comment regarding it.

https://github.com/ESCOMP/CTSM/issues/1295#issuecomment-864285821

ekluzek commented 3 years ago

The one workaround that I think I can use for this is to tell it to start one year before I want and run until one year after I want, but make sure the align year is set for it to start at the desired year. I'm going to try that out and make sure it works the way I think it should.

ekluzek commented 3 years ago

I tried my workaround, and it still fails, as it finds the same problem for the prior year, as it doesn't realize that it doesn't need the Jan/1849 data since the simulation actually starts in 1850 rather than 1849.

Here's the error I get for a MCT test case, where it uses 1849 as the first year for the stream, but starts in 1850...

SMS_D_Ld1_Mmpi-serial.1x1_smallvilleIA.IHistClm45BgcCropQianRs.cheyenne_intel.clm-cropMonthOutput

(shr_strdata_print) ---------------------------------------------------------- (shr_stream_findBounds) ERROR: limit on and rDateIn lt rDatelvd 18490101.0000000 18490116.5000000
ERROR: (shr_stream_findBounds) ERROR: rDateIn lt rDatelvd limit true

ekluzek commented 3 years ago

I realized that this is a little more complex than I was initially thinking, but I still think this is the right thing to do. The added complexity is that when you have lists of files you'll need to add in the file before the start year as well as the file just after the end of the end year. So for example if the files are monthly add the December of the year before and January of year after.

The other thing that will happen here is that answers will change (at the start and end) as compared to the "cycle" case because it will use the previous time-value rather than the end of the time-series. But, that is a much more correct thing to do for a transient historical series for example. When you are cycling over a limited set of years using the last valid date in the series is the right thing to do. But, when using a historical series, it makes the most sense to use the last time value of the previous year. When the time-series data is long using that last time value means using a pretty screwy value when you are doing something like running over 1850 to 2100. Using the December 2100 value for the 1850 startup is a pretty strange thing to do.

mvertens commented 3 years ago

@ekluzek - I think what you are suggesting makes sense - but I feel that until we implement this functionality in CDEPS we should back out the limit option and go back to the original formulation. This is impacting a series of tests that are now failing. In addition, the dlimit option should be carefully tested in a variety of configurations before we incorporate it into the CDEPS code base the way it is now.

jedwards4b commented 3 years ago

@mvertens @ekluzek The time field in stream data files is partially defined by the units attribute, for example time:units = " days since 1992-08-12 20:00:00" ; This field is read by dshr_stream_mod.F90 and used to set strm%file(k)%date(n) here.

If offset is non-zero this date may be adjusted in the code immediately following that line.

However I can't see anyplace in shr_stream_findBounds where that date is used again. It seems to me that this test should fail on startup because the first date of the file is 6 months after the first model date.

ekluzek commented 3 years ago

@mvertens I looked up what I changed to limit and it's these two commits.: a5b1e8afb303bdec93835039ee0373bbeb6ff1f1, 29a6ad338cea0ac8bb62dff161843d345ac10098. The cases it involves are: "Set limit on NEON, CLM_USRDAT, and NLDAS2 HIST"

I can make another issue to back that change out, as well as a PR for it if you like.

mvertens commented 3 years ago

That would be great. Thanks.

On Wed, Jun 23, 2021 at 10:10 AM Erik Kluzek @.***> wrote:

@mvertens https://github.com/mvertens I looked up what I changed to limit and it's these two commits.: a5b1e8a https://github.com/ESCOMP/CDEPS/commit/a5b1e8afb303bdec93835039ee0373bbeb6ff1f1, 29a6ad3 https://github.com/ESCOMP/CDEPS/commit/29a6ad338cea0ac8bb62dff161843d345ac10098. The cases it involves are: "Set limit on NEON, CLM_USRDAT, and NLDAS2 HIST"

I can make another issue to back that change out, as well as a PR for it if you like.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ESCOMP/CDEPS/issues/101#issuecomment-866972933, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4XCE43QPCWQDIKL6C53HLTUIBOHANCNFSM466NFBAA .

-- Mariana Vertenstein CESM Software Engineering Group Head National Center for Atmospheric Research Boulder, Colorado Office 303-497-1349 Email: @.***

jedwards4b commented 3 years ago

@ekluzek I'm working on fixing the limit issues, I don't think you should back out your change.

mvertens commented 3 years ago

@jedwards4b - great! That's awesome.

On Wed, Jun 23, 2021 at 10:49 AM jedwards4b @.***> wrote:

@ekluzek https://github.com/ekluzek I'm working on fixing the limit issues, I don't think you should back out your change.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ESCOMP/CDEPS/issues/101#issuecomment-867000064, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4XCE6RS7IGL4HSZ7Y2GRDTUIGBXANCNFSM466NFBAA .

-- Mariana Vertenstein CESM Software Engineering Group Head National Center for Atmospheric Research Boulder, Colorado Office 303-497-1349 Email: @.***

ekluzek commented 3 years ago

OK, thanks @jedwards4b. If you change your mind -- let me know I'm happy to do it if it helps. Would be simple to do.

In the long run I'd rather to get this right though. So I'm really glad you are working on this. Thanks so much.

jedwards4b commented 3 years ago

After careful examination and experiments with the test SMS_Ly1_Mmpi-serial_Vnuopc.1x1_vancouverCAN.I1PtClm50SpRs.cheyenne_gnu.clm-output_sp_highfreq I have determined that the limit code in dshr_stream_mod.F90 is working correctly as written.

Out of the box the test results in:

 ERROR: (shr_stream_findBounds) ERROR: rDateIn gt rDategvd limit true

which is true. The RUN_STARTDATE=2000-01-01 and the rDategvd=19920826.583368056 so the model is trying to start at a date beyond the end of the file.

If I change RUN_STARTDATE=1992-08-12 this also fails, this time with error:

   ERROR: (shr_stream_findBounds) ERROR: rDateIn lt rDatelvd limit true

also true, but a bit more subtle. The units on the time variable in the stream file are:

time:units = " days since 1992-08-12 20:00:00" ;

So in order to fall within the bounds of the file you must also change

./xmlchange START_TOD=72000

with this change the model runs to the last time of the stream file and then stops with:

  ERROR: (shr_stream_findBounds) ERROR: rDateIn gt rDategvd limit true

which I think is also correct behavior.

If I go back to RUN_STARTDATE=2000-08-12 and set yearAlign=2000 in the datm.streams.xml I get analogous behavior (this also works)

If I set START_TOD=0 and change the stream offset value to -72000 this also works as expected.

jedwards4b commented 3 years ago

@ekluzek I also tried the case mentioned in https://github.com/ESCOMP/CTSM/issues/1295 In this case with

 model_year_align_ndep = 2018
 ndep_taxmode = 'limit'

it fails with:

 ERROR: (shr_stream_findBounds) ERROR: rDateIn lt rDatelvd limit true

which is correct because the first time in the file is 15.5 and so the first date is 2018-01-15 but why do you want to align the ndep year - this setting is using the 1850 values from the file for your 2018 run. If I remove this setting from your user_nl_clm the run completes.

jedwards4b commented 3 years ago

I think that the correct settings for this case are

 model_year_align_ndep = 1850
 ndep_taxmode = 'limit'
 ndep_varlist = 'NDEP_month'
 ndepmapalgo = 'bilinear'
 stream_fldfilename_ndep = '/glade/p/cesmdata/cseg/inputdata/lnd/clm2/ndepdata/fndep_clm_f09_g17.CMIP6-SSP3-7.0-WACCM_1849-2101_monthly_c191007.nc'
 stream_year_first_ndep = 1850
 stream_year_last_ndep = 2101
ekluzek commented 3 years ago

@jedwards4b you are right that setting for the align year makes no sense. I'll have to dig into our case and figure out why that's happening. Thanks for pointing that out.

And I agree with your list of settings above. So if I start on Jan/1/1850 -- does the above settings work? I thought it would complain that first date in the file for 1850 is 15.5 days in, so limit would only work if you started at Jan/16/2850 and after?

The thing that I'd like to get to work is for it to use the December/1849 value from the file to do the time interpolation for Jan 1st through Jan/15 1850. If that's already working that would be great...

jedwards4b commented 3 years ago

I'm really confused about that - it would imply that you want your 2018 run to use the 1850 data in the file when the file does contain 2018 data and it should start from there. I think that the setting

 model_year_align_ndep = 2018

caused the beginning of the file (1849 not 1850) to align with 2018.