Global time series plots look different after CDAT replacement #600
Comments
|
Looking at https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.forsyth2/zppy_test_complete_run_www/zppy_pre_v2.5.0rc2/v2.LR.historical_0201/global_time_series/global_time_series_1850-1860_results/ (the results from before releasing v2.5.0, e.g. shortly after merging the land plot inclusion (#584) -- it appears the land plots were always blank. That wasn't caught during the testing phase. |
|
Interestingly, it appears Pre-519: Post-519: Even applying |
|
@tomvothecoder Is there anything about my code changes in #519 that would cause these plot differences? In particular, does xCDAT/xarray handle end-year boundaries (e.g., last year to be included) differently than |
|
@forsyth2 I think there two things we can check at this point.
Also, I'm not sure why the land plots were not showing up in the pre#519? |
|
Comparison of print-statement outputs Notes:
Code block 1
The answer is no, it does not. Notes:
In
Pre-519Output: Post-519Output: 20 instances of So, it looks like aside from Code block 2
The answer is no, they are not. Post-519 adds 1860 (as mentioned in #600 (comment) above too).
Pre-519Output: for each region ( Post-519Output: for each region ( Here, we see that 1860 has been added to the time list. Code block 3
Pre-519Output: Post-519Output: Comparison of variablesThe diff of We can see that the variables are not being computed as the same values for the non-ocean plots. These were computed using this script: Diff scriptWhy weren't the lands plots working pre-519?
It looks like both of the land plots were running into dimensions issues: That implies there were only 10 years but 11 data points. So, when the CDAT change made it 11 years instead of 10 years, there was suddenly a matching number of years to data points. This dimensions issue seems similar, if not identical, to the one mentioned in #571 (which is still open). This exact error is mentioned in #400 (comment), which eventually led to #571 being opened. From pre-519: |
|
@chengzhuzhang See error descriptions above. How should we proceed with this? #519 changed the global time series plots to no longer match the expected results (at https://web.lcrc.anl.gov/public/e3sm/zppy_test_resources/expected_complete_run/global_time_series/global_time_series_1850-1860_results/). That means I now can't properly test other Options:
|
Possible CDAT behaviorJust a clue for debugging: you might want to watch out for There is a "slice flag" feature that is really confusing, but the default value is What to tryYou can try running the Python debugger on the pre-519 |
Properly configuring the debuggerFor reference: to use the Unified environment in VS Code debugger:
The last line is important for debugging into other packages. DebuggingThat leads to: From https://github.com/CDAT/cdms/blob/3f8c7baa359f428628a666652ecf361764dc7b7a/Lib/cdmsNode.py#L468, we know: Here, we have the following in the variable explorer: I'm wondering if that's the 11 values for the time series variable. For
The I don't see anything here that would suggest 11 years of data(e.g., 121 for 1 month on the end or 132 for a full extra 12 months). So, my conclusion is that the pre-519 (with CDAT, using |
|
In an effort to further isolate the buggy code I wrote the following script: Pre/Post-519 comparison scriptWe can see that the diff of the y values in the plots matches what I listed above in #600 (comment). This suggests this stand-alone script is written correctly. Aside from being stand-alone, this script also uses the same input directory for both pre- and post-519 code. We can see that from the very beginning, the pre-519 code is working with 120 months (10 years) while the post-519 code is working with 11 years. @tomvothecoder @chengzhuzhang This confirms for sure that |
|
Results with debugger: pre-519
post-519
That brings us to Before, Before that line, we call the When we get to this function, Following the
|
|
In It appears that in sets So, something wrong is happening in It looks like this block in is setting The debugger won't let me go further into However, if we look deeper into So the issue lies much earlier than this -- when the |
|
In Here, Going further up: Here, Going further up: Going further up: The Going further up:
However, I'm now able to pull more and more code outside the xcdat/xarray package, for easier debugging: I now see that the slicing happens in I now have: So, getting the "time" attribute of "PRECC" seems to be slicing "PRECC" incorrectly... I'm trying to debug why that would happen. My best guess so far is something about the seasons handling winter as |
|
@forsyth2 thanks for creating the standalone code for debugging, I took it and further simplified. Please see below: You can play with this to show why there is difference in your implement. In brief, the EAM output has time value recorded at the end of the time interval. While |
|
In Before this line is called, So, we can work earlier into the code now: By the time we get to this line in we have This appears even earlier in the code. By the time we get to the following line in we have |
|
@chengzhuzhang Ah thank you, I will look into |
@chengzhuzhang That issue was carried over to #571. Are you saying that using |
I meant that #400 has a similar script for resolving a similar issue. It also explained the rational behind time averaging. It is possible that with |
By default, xCDAT will behave the same way as CDAT by using the existing time bounds to create the temporal weights. Basically, I think the only difference here is the CDAT is centering time coordinate automatically while xCDAT does not. Centering time coordiante with xCDAT might resolve the differences, but I'm not exactly sure. |
|
@tomvothecoder thank you for chiming in! After some experiment (e.g. saving time weights from xcdat), I think we can confirm that centering time should be the right solution. Without centering time, the xcdat codes would create 11 annual mean values with the last time point 1860-01-01, that has a time weights of 1.0. In the case xCDAT implementation, since xCDAT won't center time automatically by default as CDAT, center_times=True is needed to get the correct month and weights. |
The plots also look correct when I implement |
Request criteria
Issue description
It appears the merging of #519 altered the appearance of the global time series plots. I ran the complete-run test on
mainand it showed differences for global time series.Before merging (these plots also match the expected plots in the complete-run test): https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.forsyth2/zppy_test_complete_run_www/test-2.5.0/v2.LR.historical_0201/global_time_series/global_time_series_1850-1860_results/
After merging: https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.forsyth2/zppy_test_complete_run_www/test-main-20240611/v2.LR.historical_0201/global_time_series/global_time_series_1850-1860_results/
Original plots: the plots seem similar but definitely not identical. The large jump at the end of the post-519 graphs make me think #519 broke something rather than fixed something.
Land plots: There must have been some mistake in updating the expected Land plots prior to releasing 2.5.0 -- the expected plots show nothing plotted.... The post-519 plots actually have results. That said, these results are clearly incorrect, considering they're for the most part just horizontal lines.
The text was updated successfully, but these errors were encountered: