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

Feature 2654 ascii2nc polar buoy support #2846

Merged
merged 17 commits into from
Apr 10, 2024

Conversation

davidalbo
Copy link
Contributor

@davidalbo davidalbo commented Mar 27, 2024

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

  • 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.

  • 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/0README and doing something similar.

  • 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.

  • 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.

  • 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


  • Will this PR result in changes to existing METplus Use Cases? [No]

    At least I don't think so.

  • 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 ⚙️ 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.

@davidalbo davidalbo added the component: user support User support issue label Mar 27, 2024
@davidalbo davidalbo added this to the MET 12.0.0 milestone Mar 27, 2024
@davidalbo davidalbo self-assigned this Mar 27, 2024
@JohnHalleyGotway
Copy link
Collaborator

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

@JohnHalleyGotway
Copy link
Collaborator

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 JohnHalleyGotway marked this pull request as draft March 28, 2024 20:43
@davidalbo davidalbo marked this pull request as ready for review March 29, 2024 21:25
@JohnHalleyGotway JohnHalleyGotway removed the component: user support User support issue label Apr 8, 2024
…2nc_polar_buoy_support

Also update ascii2nc/Makefile.in by running bootstrap on seneca.
And resolve conflicts with the Ascii2NC enumeration in the develop
branch.
@JohnHalleyGotway
Copy link
Collaborator

JohnHalleyGotway commented Apr 9, 2024

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.

…all the python scripts that should be installed.
@JohnHalleyGotway
Copy link
Collaborator

JohnHalleyGotway commented Apr 9, 2024

Keeping track of my PR review steps here:

  • 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.
  • Ran bootstrap on seneca for this feature branch to recreate src/tools/other/ascii2nc/Makefile.in to include compiling the new IABP code.
  • 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.
  • Reviewed the doc updates in reformat_point.rst.
  • 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.
  • 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.
  • 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.
  • 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:
      Screen Shot 2024-04-09 at 5 05 16 PM'
  • 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.

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

@JohnHalleyGotway JohnHalleyGotway deleted the feature_2654_ascii2nc_polar_buoy_support branch December 19, 2024 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: 🏁 Done
Development

Successfully merging this pull request may close these issues.

Enhance ASCII2NC to support IABP/IPAB Arctic and Antarctic drifting buoy observations
2 participants