dbekaert / RAiDER

Raytracing Atmospheric Delay Estimation for RADAR
Apache License 2.0
70 stars 39 forks source link

NCUM in Circle CI #271

Closed dbekaert closed 3 years ago

dbekaert commented 3 years ago

@kam3545 we would like to add the NCUM model to the circle ci suite. It would allow us to test for each code change that the model still works and when it fails allows the team to go in and adress the issue.

For this we would need sample files for NCUM. We do not need to complete model file, a small subset would be sufficient.

kam3545 commented 3 years ago

@dbekaert , I am attaching one sample subset file for your reference. Last time, Dr. Yang suggested to add filename for pre-downloaded NCUM files. (_If you’d like to use the pre-downloaded data as I did for the test, you can simply add “filename = ‘.’ + filename” at Line #79 and put the folder “weatherfiles” one level above RAiDER. That should work temporarily and produce the debugging plots as I sent earlier.) This option is not working now. testncmr.zip

dbekaert commented 3 years ago

@leiyangleon while adding in the scenario 1 test, could you at the same time look at this for NCUM too? Similar likely needs a small fix wrt issue #270

leiyangleon commented 3 years ago

@dbekaert @kam3545 The error should be fixed by this PR #281.

However, for using a small dataset in CI test, the current NCMR reader does not support it. Because NCMR files (in netCDF format) normally cover the whole globe (lat -90-->90 and lon 0-->360). The indices in lat/lon pixels are determined by using the start/end convention and the bounding box user specifies. The small test dataset above, however, has already been truncated from the whole globe with lons converted to -180-->180 as well, all of which will mess up the index calculation in the current NCMR reader.

So, PR #281 guarantees that the NCMR reader works well for the typical NCMR files (the whole globe) NOT a small piece of those files. I am working on an updated version of the reader which is able to handle small dataset.

leiyangleon commented 3 years ago

@kam3545 @dbekaert BTW, the use of the NCMR reader now should be in compliance with other models. Ignore the temporary method mentioned before. Just put the pre-downloaded NCMR nc file into RAiDER/weather_files/ where all other model data are stored, the program will run through without problems.

dbekaert commented 3 years ago

@leiyangleon Thanks, reading up on this here now. So ignore my comment in the PR.

@kam3545 Per description from @leiyangleon we would need to fetch the full product to make the test suite works. We generally do not add large files to the repo as this makes it too heavy. Is there a public website were we could directly fetch a product from for unit testing, e.g. an ftp or another staged area?

kam3545 commented 3 years ago

@dbekaert @leiyangleon Thanks, The changes are working fine for NCMR data. Today, I will check other RAiDER utilities with NCMR and confirm you again.

@dbekaert NCMR data are available with us in a local server (2016 onwards). I will find out possibility to upload on a ftp server for testing.

Thanks to RAiDER team for supports. Prashant

kam3545 commented 3 years ago

@dbekaert Thanks, now I am able to read NCMR files in RAiDER. Found few more concern in package. Raise another one issue online, soon include others also. I put sample files on our ftp, details I will email you.

leiyangleon commented 3 years ago

closing this as has been solved by PR #297.