Feature/ Resolved Issues and Added Tests to SMOPS Ver4 Reader

NASA-LIS / LISF

Land Information System Framework

Apache License 2.0

117 stars 155 forks source link

Feature/ Resolved Issues and Added Tests to SMOPS Ver4 Reader #1544

Closed ejalilva closed 3 months ago

ejalilva commented 4 months ago

Changes in this Pull Request

In create_SMOPS_ASCATsm_filename subroutine:
- Updated the Ver4_blended_time year to 2024 from 2224.
- Updated the if statement to select the file version based on the date.
In read_SMOPS_ASCAT_data subroutine:
- Moved the immature GRIB message release to after checking for SMOPS Metop-C data.
- Added logical variables a_exist, b_exist, and c_exist to check the existence of Metop-A, B, or C based on the file version.
- Added conditions to read SMOPS Metop-A and B data for version 3 and before, and SMOPS Metop-B and C for version 4.
- Added conditions to read SMOPS Metop-A and B QA data for version 3 and before, and SMOPS Metop-B and C QA data for version 4.
- Added an if statement to conditionally apply the 6-hour time offset based on the file version (add offset for version 3 and do not add it for version 4)
Global testcase:

DA run: /discover/nobackup/ejalilva/SHARE/SMOPS_testcase/LIS_run

emkemp commented 4 months ago

Thanks @ejalilva. I expect to work on this Wed afternoon.

emkemp commented 4 months ago

Hi @ejalilva

Can you please open group permissions to /discover/nobackup/ejalilva/SHARE/? I cannot ls or cd into it.

ejalilva commented 4 months ago

Hi @emkemp the group permission is open now, the LSM cdf file is yet to be added can you try running it on Thursday morning?

emkemp commented 3 months ago

So.... I just did a fresh clone/fetch of this pull request, and I'm still getting zero diffs between the SMOPS DA and the data denial (hidden SMOPS) cases. Is this pull request the latest version of the code? Alternatively @jvgeiger have you had better test results?

jvgeiger commented 3 months ago

I just began my testing.

emkemp commented 3 months ago

I also notice an apparent typo in the lis.config.

SMOPS ASCAT naming convention: AGRMET ops

I'm going to try another run with quotation marks.

jvgeiger commented 3 months ago

Yes. That needs quotation marks.

jvgeiger commented 3 months ago

Is this pull request the latest version of the code?

Yes. This is the latest. Ehsan and I just checked.

ejalilva commented 3 months ago

@emkemp Please also compare the next time step output (LIS_HIST_202405011200) because based on the log file in both previous and patched version all the assimilation are performed after writing the first time step

emkemp commented 3 months ago

@ejalilva I diff'ed all three output fimes (00Z 1 May, 12Z 1 May, 00Z 2 May). All are zero diff.

emkemp commented 3 months ago

Adding the quotation marks in the lis.config doesn't change the answer.

emkemp commented 3 months ago

So, @jvgeiger and I have compared our lis.config files. It appears the test case changed after I copied files over. I'm not using the right restart file, attributes files, or CDF files. So, I will update and run again.

@jvgeiger does see differences when he runs w/ and w/o SMOPS v4, which suggests the code works.

ejalilva commented 3 months ago

Thanks for checking @emkemp yes that's correct, I replaced all of those files with Test cases for NRT_GLOBAL that you provided. still I am wondering why the result was identical in your runs because even with the previous rst, cdf and etc. everything was similar in the two runs except for smops assimilation, so the result should have been different.

emkemp commented 3 months ago

Okay, after starting over with a fresh copy of the test case, I can reproduce results. I also see differences between the DA and data denial tests. I am satisfied, and will merge the code.

Thank you for bearing with me.

ejalilva commented 3 months ago

Of course, thanks for offering data denial test that helped identifying time offset issue!