Feature 2654 ascii2nc polar buoy support

davidalbo commented 3 months ago

Expected Differences

[ ] Do these changes introduce new tools, command line arguments, or configuration file options? [Yes]

Two new python scripts have been added: scripts/python/utility/find_iabp_in_timerange.py looks at all the iabp files and lists those in a range of times scripts/python/utility/get_iabp_from_web.py pulls all the iabp files from the web.

Command line arguments for ascii2nc have been changed to optionally include a time range, same format as other apps:

[-valid_beg time]
[-valid_end time]

"-valid_beg time" in YYYYMMDD[_HH[MMSS]] sets the beginning of the processed data time window (optional).
"-valid_end time" in YYYYMMDD[_HH[MMSS]] sets the end of the processed data time window (optional).

[ ] Do these changes modify the structure of existing or add new output data types (e.g. statistic line types or NetCDF variables)? [No]

Pull Request Testing

[x] Describe testing already performed for these changes: Both python scripts were run to get and parse the iabp data files for time ranges. ascii2nc was then run on a set of files that contain a particular time range, with the command line option to specify time range used. The resultant netcdf file was looked at using ncdump. Notes on all of this, along with all the iabp data files, are on seneca in /d1/personal/dave/ascii2nc_test. Also, the unit test was expanded to include the same specific ascii2nc input data and command line arguments.
[x] Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions: I'd recommend starting by reading /d1/personal/dave/ascii2nc_test/0READMEand doing something similar.
[x] Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes] The document now describes the IABP option, and the optional time range command line args.
[x] Do these changes include sufficient testing updates? [Maybe] The use of the time range command line args was not tested on other ascii2nc formats, only on the iabp data format.
[x] Will this PR result in changes to the MET test suite? [Yes]
If yes, describe the new output and/or changes to the existing output: The ascii2nc unit test now includes an iabp data test, with an output file: &OUTPUT_DIR;/ascii2nc/iabp_20140101_20140201.nc
[x] Will this PR result in changes to existing METplus Use Cases? [No]
At least I don't think so.
[x] Please complete this pull request review by [Fill in date].

Pull Request Checklist

See the METplus Workflow for details.

[ ] Review the source issue metadata (required labels, projects, and milestone).
[ ] Complete the PR definition above.
[ ] Ensure the PR title matches the feature or bugfix branch name.
[ ] Define the PR metadata, as permissions allow. Select: Reviewer(s) and Development issue Select: Milestone as the version that will include these changes Select: Coordinated METplus-X.Y Support project for bugfix releases or MET-X.Y.Z Development project for official releases
[ ] After submitting the PR, select the :gear: icon in the Development section of the right hand sidebar. Search for the issue that this PR will close and select it, if it is not already selected.
[ ] After the PR is approved, merge your changes. If permissions do not allow this, request that the reviewer do the merge.
[ ] Close the linked issue and delete your feature or bugfix branch from GitHub.

JohnHalleyGotway commented 3 months ago

Please document the new python utilities in the ASCII2NC chapter with a level of detail similar to this section.

JohnHalleyGotway commented 3 months ago

For each line of the input file (i.e. unique lat/lon and time combination), write an output obs named location with a value of 1 to indicate the buoy location. Additionally, write other obs for non-missing temperature data.

JohnHalleyGotway commented 3 months ago

I see that the 2 new scripts in the scripts/python/utility directory are NOT actually being copied over into the installed share/met/python/utility directory because Makefile.am in the utility directory was not updated to install them. I added that logic for this feature branch with this commit.

JohnHalleyGotway commented 3 months ago

Keeping track of my PR review steps here:

[x] Merged latest from develop into this feature branch - since I'd taken too long to review this!!! And fix conflicts with the changes to the FileType enumeration.
[x] Ran bootstrap on seneca for this feature branch to recreate src/tools/other/ascii2nc/Makefile.in to include compiling the new IABP code.
[x] Updated Makefiles in scripts/python/utility to actually install the python scripts that live there.
- @JohnHalleyGotway removed the unreachable (and unused) code that was triggering the SonarQube bug.
[x] Reviewed the doc updates in reformat_point.rst.
- @JohnHalleyGotway fixed this copy/paste error in the docs on 4/10/24.
[x] Run the installed Python utility scripts to make sure they work as described.
- Ran python3 scripts/python/utility/get_iabp_from_web.py -o iabp_data. It takes a while to run. I killed it after pulling 633 files, but that's fine.
- Ran python3 scripts/python/utility/find_iabp_in_timerange.py -s 20240801 -e 20240901 -d iabp_data to extract 1 month's worth of data. But that found no results. After doing some grepping, I decided to pull 2020 data instead.
- Ran python3 scripts/python/utility/find_iabp_in_timerange.py -s 20200101 -e 20201231 -d iabp_data. That printed 13 files which contain buoy data for 2020.

[x] Test ASCII2NC with the IABP inputs:

Ran this command:

bin/ascii2nc -format iabp \
iabp_data/132461.dat iabp_data/39.dat iabp_data/44.dat iabp_data/52959.dat iabp_data/52960.dat \
iabp_data/52967.dat iabp_data/52988.dat iabp_data/53001.dat iabp_data/53005.dat \
iabp_data/53084.dat iabp_data/53086.dat iabp_data/53093.dat iabp_data/53095.dat \
iabp.nc -log run_iabp.log -v 3

And it worked fine!

DEBUG 2: Finished processing 276941 observations for 122269 headers.

[x] Test the newly added ASCII2NC -valid_beg and -valid_end command line options to confirm they work as expected.
- Check that it's additive using:
- -valid_beg 20200601_123456 yields 56543 observations for 26895 headers.
- -valid_end 20200601_123456 yields 220398 observations for 95374 headers.
- 220398 + 56543 = 276941, as expected!
- Check begin and end times:
- Adding -valid_beg 20200101 -valid_end 20201231:
```
DEBUG 2: Finished processing 120917 observations for 57333 headers.
```
- So time filtering works well.
[x] Run the output through plot_point_obsto make sure it looks reasonable.
- bin/plot_point_obs iabp_2020.nc iabp_2020.ps and this definitely puts buoys near the arctic: '

[x] Run the output through the print_pointnc2ascii.py script to make sure it looks reasonable:

/nrit/ral/met-python3/bin/python3 scripts/python/utility/print_pointnc2ascii.py iabp_2020.nc > iabp_2020.txt

head -5 iabp_2020.txt and the output looks good:

IABP_STANDARD 132461 20200101_000000  72.4807 -110.9611 NA Location     1005.2000 NA NA   1.00000000
IABP_STANDARD 132461 20200101_000000  72.4807 -110.9611 NA Temp_surface 1005.2000 NA NA -16.13999939
IABP_STANDARD 132461 20200101_000000  72.4807 -110.9611 NA Temp_air     1005.2000 NA NA  11.52000046
IABP_STANDARD 132461 20200101_010000  72.4807 -110.9613 NA Location     1004.5000 NA NA   1.00000000
IABP_STANDARD 132461 20200101_010000  72.4807 -110.9613 NA Temp_surface 1004.5000 NA NA -16.13999939

I see that the elevation, height, and qc flags are all stored as bad data (NA), which is probably fine. Perhaps they could be 0 instead, but I don't have a strong feeling about that.

[x] Review the newly added ASCII2NC unit test and inspect the output.
[x] Review the code changes for clarity and consistency.

dtcenter / MET