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

2024.01-alpha2 stdout is swamped by a warning increasing its size 1000 fold #1430

Closed
nikizadehgfdl opened this issue Dec 19, 2023 · 6 comments

Comments

@nikizadehgfdl
Copy link
Contributor

Describe the bug
In the tag 2024.01-alpha2 stdout is swamped by the following warning for regression tests

 WARNING from PE  1807: data_override: current time outside bounds, use [previous]:current:[next] files in data_table

To Reproduce
Run ESM4.2 regression test

Expected behavior
Should be no such warning or at most a NOTE or FATAL if this is a problem

Additional context
The code is newly added to main branch by PR #1408 . There are many lines of code with this warning

https://github.com/NOAA-GFDL/FMS/pull/1408/files#diff-482d9c64ab36090ff35a1709f8ad19050a7d6817855d6a2d897096bfc51e0f36R845

@nikizadehgfdl
Copy link
Contributor Author

Anyone knows what is wrong with this logic and why it is tripped in seemingly fine regression tests?

 if ((time<first_record) .or. (time>last_record)) then
         call mpp_error(WARNING, &
              'data_override: current time outside bounds, use [previous]:current:[next] files in data_table')
  endif

@nikizadehgfdl
Copy link
Contributor Author

@raphaeldussin we should fix this. This check was not in the original code, perhaps it should not be in the new code either?

@nikizadehgfdl
Copy link
Contributor Author

I got curious and put print statements to see what's going on. Indeed the logic above is tripped thousands of times with values like

time:                day=0, sec=0, ticks=0 
first_record:   day=15, sec=0, ticks=0
last_record:    day=350, sec=0, ticks=0

or

time:                day=0, sec=7200, ticks=0
first_record:   day=693499, sec=64800, ticks=0 
last_record:    day=693499, sec=64800, ticks=0

I don't know what is going on. But given that models run with these warnings, we should remove the if blocks and let it be like it was before. Ignorance is bliss.

@raphaeldussin
Copy link

@nikizadehgfdl This sounds dangerous. Removing the if blocks would not let the user know what is the problem with the time axis and how to fix it. This logic should work. Do we allow the current time to be outside of the time bounds or are we getting bad time bounds in the tests ?

@nikizadehgfdl
Copy link
Contributor Author

nikizadehgfdl commented Dec 20, 2023

In ESM4 case most of these warnings are because the data file has only 1 time level at 12/31/1899, (I guess that's where day=693499 comes from above). I think time_interp handles constant in time source data.
There is another file which has only 12 time levels (I guess corresponds to first_record day=15 and last record day 350 above). Again time_interp is smart enough to know user meant shifting the source time to model year.

@rem1776
Copy link
Contributor

rem1776 commented Jan 8, 2024

fixed with #1431

@rem1776 rem1776 closed this as completed Jan 8, 2024
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

No branches or pull requests

3 participants