UA SWE/SNWD Reader for LVT #129

rkim3 · 2019-05-29T15:43:24Z

This update adds a reader to LVT that reads in University of Arizona SWE/ Snow depth data.

I've added test cases for this reader : /discover/nobackup/rkim3/gitLIS/testcase/UA_SWE
/discover/nobackup/rkim3/gitLIS/testcase/UA_SNWD

Let me know if you have any other questions. Thanks!

dmocko · 2019-05-31T18:08:10Z

Hi,

Thanks much for submitting this pull request for this needed feature.
Thanks also for creating and sharing testcases.

I've started to review and test, and I have three requests of you:

My code compiled, but when I ran, I received this error:

Error************
observation init routine for source UA SWE is not defined
please see the configs/lvt.config.master file.....
program will seg fault.....
Error************

You likely edited these two files:
lvt/plugins/LVT_datastream_pluginMod.F90
lvt/plugins/LVT_pluginIndices.F90

However, they were not in the commit and pull request.
Can you please also commit your edits to these files?

Can you please also add the required documentation
for using "UA SWE" and "UA SNWD" to the this file?
lvt/configs/lvt.config.master
Can you modify both of your testcases to generate
tar files as described on this page?
https://github.com/NASA-LIS/LISF/blob/master/docs/howto_create_lis_testcases/howto_create_lis_testcases.adoc

The important thing is to include all the necessary
files (but not any more than necessary) into the tar
files as described. Don't include any testcase files
in your commit, as detailed in the link.

Thank you!

David

rkim3 · 2019-06-04T19:38:32Z

Hi,

Thanks for your reviewing.
Here are my responses to your requests:

I've committed all my edits based on your requests Updates for compiling LISF #1 and Bug in the GLASS LAI reader #2.

lvt/plugins/LVT_datastream_pluginMod.F90
lvt/plugins/LVT_pluginIndices.F90
lvt/configs/lvt.config.master
I've modified both my testcases with tar files.

Let me know if you have further questions.

Thanks,
Rhae Sung

dmocko · 2019-06-07T13:10:32Z

Hi,

Thanks much for the code commits. I can now successfully compile LVT.

I didn't realize this until I started running your testcases, but there is only one data file from UA for both the SWE and SnowDepth variables. Thus, if would be better if there was only one reader for them both, instead of separate instances of "UA_SWE" and "UA_SNWD".

Instead of separate directories in lvt/datastreams, and separate plugins, can you please change to code so there is only "UA_Snow" or a similar name?

Look at other datastreams (such as FLUXNETmte) for how to have LVT process more than one variable in a single datastream through LVT.

It would be best if you worked off an updated master branch to add this "feature/UA_Snow" code, as there are now conflicts as well.

Sorry for this trouble, but it will be worth it for users to be able to analyze both SWE and SnowDepth in one LVT instance.

No need for updating any testcase, though. Please just update and commit the code, and I will take another look at it.

David

see also NASA-LIS#129

Resolves the conflicts

dmocko · 2019-06-20T20:15:16Z

Just a short update to say that I'm still reviewing this pull request. I will have some code commits as well to refine some items.

rkim3 · 2019-06-20T20:24:11Z

Thanks, David!

This commit makes some code fixes and modifications for the LVT UA SNOW reader. There are two main changes: - The valid time of the observations are approximately 12Z. This info is from the data producers at Univ. of Arizona, and is based on the 12Z observation time of many of the in situ surface observations. - Only one annual water year file is read with each call to the read routine. Previously, both the current water year and the next water year file were read, but only the data for the current day was used. The code was modified so only the current day's file is read. This results in quicker execution from cutting down on the number of reads. See: NASA-LIS#129

dmocko · 2019-06-25T14:37:58Z

Hi Rhae Sung,

I just committed some updates. Can you update your code with these updates, and then generate a small testcase? Just a month or so, not the entire record. Perhaps across a water year transition. Please also process both SWE and SnowDepth in the same testcase:

LVT datastream attributes table::
SWE 1 1 kg/m2 - 1 1 SWE 1 1 kg/m2 - 1 1
SnowDepth 1 1 m - 1 1 SnowDepth 1 1 m - 1 1
::

Bundle them as before, with only including the data necessary to run and evaluate this particular testcase. After you have them, I will also test, and then we can approve this pull request.

David

rkim3 · 2019-06-26T17:19:19Z

Hi David,

It looks like LVT is trying to read LIS output for every hour. This happened even after choosing different computation frequencies (e.g., 3hr, 1da):

[INFO] LVT cycle time: 09/01/2009 11:00:00
[WARN] LIS file
/discover/nobackup/projects/SEUP/rkim3/SEUP/NoahMP/MERRA2/OL/OUTPUT/SURFACEMODE
L/200909/LIS_HIST_200909011100.d01.nc does not exist
[INFO] LVT cycle time: 09/01/2009 12:00:00
[INFO] Reading LIS output
/discover/nobackup/projects/SEUP/rkim3/SEUP/NoahMP/MERRA2/OL/OUTPUT/SURFACEMODE
L/200909/LIS_HIST_200909011200.d01.nc
[INFO] Reading UA data
/discover/nobackup/projects/lis/STN_DATA/UA_SNOW/4km_SWE_Depth_WY2009_v01.nc
[INFO] LVT cycle time: 09/01/2009 13:00:00
[WARN] LIS file
/discover/nobackup/projects/SEUP/rkim3/SEUP/NoahMP/MERRA2/OL/OUTPUT/SURFACEMODE
L/200909/LIS_HIST_200909011300.d01.nc does not exist

Could you double check/test this?

Rhae Sung

dmocko · 2019-06-26T18:18:33Z

Hi,

Thanks for reporting this, but I don't think that this is an issue.
LVT often does this when the interval is less than the interval of the LIS output data.

LVT only calculates the comparison metrics for when there is data at the same time.

What values do you have for "Metrics computation frequency": "Metrics output frequency:", and "LIS output interval:"?

It looks like you only have LIS data on 12Z.

Please check your output, but I suspect that it will be correct as you have expected, based on your settings, despite the "does not exist" messages.

David

rkim3 · 2019-06-26T18:56:24Z

Hi,

This is not that case. I have 3hr-interval LIS output and my settings are:

Metrics computation frequency: 1da
Metrics output frequency: 1da
LIS output interval: 3hr

I also tried to use a '3hr' metrics computation frequency value, but LVT still attempted to read LIS output hourly.

I agree that "this is not an issue because LVT only calculates the comparison metrics for when there is data at the same time".

Rhae Sung

dmocko · 2019-06-26T20:19:40Z

Rhae Sung,

Thanks for the info. Based on your settings, I expect that LVT will do the following:

Calculate the daily average of the LIS output (the average of the 3-hourly data for that day.
Calculate the daily average of the UA SNOW data (which is only available once a day at 12Z).
Calculate the METRICS between these daily averages.

If you turned on TS file output in the METRICS file, you should have daily output files of these calculations.

Please compare (outside of LVT) the source datasets against the LVT output to confirm that this is correct. Or, bundle up the testcase, and I can also check.

If you only want to compare your 12Z LIS output with the 12Z UA SNOW data, instead of daily averages, then you'll need to adjust your settings. We can take that discussion offline.

David

rkim3 · 2019-06-27T14:45:20Z

Eric,

Thanks for your explanation.

I compared the source datasets against the LVT outputs, and those look good. You can also check the testcase here:

/discover/nobackup/rkim3/gitLIS/testcase/test

Rhae Sung

dmocko · 2019-06-27T19:50:56Z

Great! Thanks, this testcase works for me, and I match your target output.

Since I wrote a portion of the code, I think it's worth having @emkemp also quickly verify this testcase. If it looks good to him, one of us will approve the pull request.

emkemp

(Revised review -- documentation is correct.)

I compiled this with Intel 18, and I get the following run-time error:

Error************
observation init routine for source UA SWE is not defined
please see the configs/lvt.config.master file.....
program will seg fault.....
Error************

Did you remember to update the plugins and commit them?

Also, where is NOAHMP_OUTPUT_LIST.TBL? This is listed in the lvt.config test file, but I don't see it in the testcase directory.

Used wrong test case.

emkemp

(Second review attempt--now with the up-to-date test case!)

I compiled the code with Intel 18 and GNU 7.3. The Intel test runs have no issues, and I can reproduce the target output. However, the GNU test fails at the very end of the run, with a cryptic error message:

[ERR] return error in LVT call, Prgm Stopping...

The netCDF output file is opened but nothing is written to the file.

We/I will have to dig into this and fix the problem before the pull request can be accepted.

emkemp

Third review.

I determined that the runtime error with the GNU compiler is not associated with these changes, but is instead a bug in how LVT produces netCDF files. After putting in a test fix, the GNU test run reproduces the target results. Rerunning the Intel 18 tests also shows no new errors.

I reviewed the code and have no objections.

Given this, I now approve the pull request.

rkim3 added 2 commits May 23, 2019 15:22

Add UA SWE/SNWD data reader

ff2d922

Add UA SWE/SNWD data reader for LVT

175bb7c

dmocko self-assigned this May 31, 2019

dmocko added the enhancement New feature or request label May 31, 2019

Add UA SWE/SNWD data reader for LVT

e06766c

rkim3 added 3 commits June 12, 2019 15:48

Merging SWE and Snow depth into one plugin

75bb41b

see also NASA-LIS#129

Merge branch 'master' into feature/UA_SWE

b6a3544

Resolves the conflicts

Cleaned up lvt.configs.master

c7c0e51

dmocko requested a review from emkemp June 27, 2019 19:51

emkemp previously requested changes Jun 27, 2019

View reviewed changes

emkemp reviewed Jun 28, 2019

View reviewed changes

emkemp approved these changes Jun 28, 2019

View reviewed changes

emkemp merged commit df568b4 into NASA-LIS:master Jun 28, 2019

rkim3 deleted the feature/UA_SWE branch July 1, 2019 14:51

bmcandr added the AddToTestSuite label Apr 9, 2020

jvgeiger removed the AddToTestSuite label May 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UA SWE/SNWD Reader for LVT #129

UA SWE/SNWD Reader for LVT #129

rkim3 commented May 29, 2019

dmocko commented May 31, 2019

rkim3 commented Jun 4, 2019

dmocko commented Jun 7, 2019 •

edited

Loading

dmocko commented Jun 20, 2019

rkim3 commented Jun 20, 2019

dmocko commented Jun 25, 2019 •

edited

Loading

rkim3 commented Jun 26, 2019

dmocko commented Jun 26, 2019

rkim3 commented Jun 26, 2019

dmocko commented Jun 26, 2019

rkim3 commented Jun 27, 2019

dmocko commented Jun 27, 2019

emkemp left a comment •

edited

Loading

emkemp left a comment

emkemp left a comment

UA SWE/SNWD Reader for LVT #129

UA SWE/SNWD Reader for LVT #129

Conversation

rkim3 commented May 29, 2019

dmocko commented May 31, 2019

rkim3 commented Jun 4, 2019

dmocko commented Jun 7, 2019 • edited Loading

dmocko commented Jun 20, 2019

rkim3 commented Jun 20, 2019

dmocko commented Jun 25, 2019 • edited Loading

rkim3 commented Jun 26, 2019

dmocko commented Jun 26, 2019

rkim3 commented Jun 26, 2019

dmocko commented Jun 26, 2019

rkim3 commented Jun 27, 2019

dmocko commented Jun 27, 2019

emkemp left a comment • edited Loading

Choose a reason for hiding this comment

emkemp left a comment

Choose a reason for hiding this comment

emkemp left a comment

Choose a reason for hiding this comment

dmocko commented Jun 7, 2019 •

edited

Loading

dmocko commented Jun 25, 2019 •

edited

Loading

emkemp left a comment •

edited

Loading