-
Notifications
You must be signed in to change notification settings - Fork 24
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 #2476 tc_pairs_diag take 2 #2705
Conversation
…_DIAG_RT sources. If left empty, the ATCF ID of the corresponding tracks is read from the CIRA diagnostics input file.
…mputation of a track consensus.
…ues for consensus track. SL. ci-skip-all
…es after initializing it with the first track point.
… a value based on the diag name. SL ci-skip-all
…some actual values. SL ci-skip-all
…g_name(). Added some debugging. SL
…lues. SL. ci-skip-all
…into feature_2476_tc_pairs_diag
Co-authored-by: John Halley Gotway <johnhg@ucar.edu>
Co-authored-by: John Halley Gotway <johnhg@ucar.edu>
Co-authored-by: John Halley Gotway <johnhg@ucar.edu>
Co-authored-by: John Halley Gotway <johnhg@ucar.edu>
Co-authored-by: John Halley Gotway <johnhg@ucar.edu>
Co-authored-by: John Halley Gotway <johnhg@ucar.edu>
Co-authored-by: John Halley Gotway <johnhg@ucar.edu>
Co-authored-by: John Halley Gotway <johnhg@ucar.edu>
Co-authored-by: John Halley Gotway <johnhg@ucar.edu>
Co-authored-by: John Halley Gotway <johnhg@ucar.edu>
… loop that calculates and stores the consensus diag value, modified to store missing values as well as non-missing values. SL
…own below the call to derive_consensus(). We were filtering out the consensus members prior to using them to define the consensus. Also update the revision history.
I'll note that this GHA run did flag a difference in the tc-pairs output. However, it is just a difference in the order of the output. I downloaded/unpacked the diffs and ran:
After sorting the output, the diffs go away. The derived |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these changes look good to me.
I reviewed the PR and I approve of the changes. @JohnHalleyGotway looked at the differences in the workflow testing data and it was just a difference in the order of the data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the code changes look reasonable and make sense.
Co-authored-by: jprestop <jpresto@ucar.edu> Co-authored-by: Seth Linden <linden@seneca.rap.ucar.edu> Co-authored-by: John Halley Gotway <johnhg@ucar.edu> Co-authored-by: Daniel Adriaansen <dadriaan@ucar.edu> Co-authored-by: John and Cindy <halleygotway@Halleys-Mac-mini.local> Co-authored-by: rgbullock <bullock@ucar.edu> Co-authored-by: Randy Bullock <bullock@seneca.rap.ucar.edu> Co-authored-by: Dave Albo <dave@seneca.rap.ucar.edu> Co-authored-by: Howard Soh <hsoh@seneca.rap.ucar.edu> Co-authored-by: George McCabe <23407799+georgemccabe@users.noreply.github.com> Co-authored-by: hsoh-u <hsoh@ucar.edu> Co-authored-by: MET Tools Test Account <met_test@seneca.rap.ucar.edu> Co-authored-by: Seth Linden <linden@ucar.edu> Co-authored-by: lisagoodrich <33230218+lisagoodrich@users.noreply.github.com> Co-authored-by: davidalbo <dave@ucar.edu> Co-authored-by: Lisa Goodrich <lisag@ucar.edu> Co-authored-by: metplus-bot <97135045+metplus-bot@users.noreply.github.com> Co-authored-by: j-opatz <59586397+j-opatz@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Jonathan Vigh <jvigh@ucar.edu> Co-authored-by: Tracy Hertneky <39317287+hertneky@users.noreply.github.com> Co-authored-by: David Albo <dave@ucar.edu> Co-authored-by: Dan Adriaansen <dadriaan@ucar.edu> fix 2518 dtypes appf docs (#2519) fix 2531 compilation errors (#2533) fix #2531 compilation_errors_configure (#2535) fix #2514 develop clang (#2563) fix #2575 develop python_convert (#2576) Fix Python environment issue (#2407) fix definitions of G172 and G220 based on comments in NOAA-EMC/NCEPLIBS-w3emc#157. (#2406) fix #2380 develop override (#2382) fix #2408 develop empty config (#2410) fix #2390 develop compile zlib (#2404) fix #2412 develop climo (#2422) fix #2437 develop convert (#2439) fix for develop, for #2437, forgot one reference to the search_parent for a dictionary lookup. fix #2452 develop airnow (#2454) fix #2449 develop pdf (#2464) fix #2402 develop sonarqube (#2468) fix #2426 develop buoy (#2475) fix 2596 main v11.1 rpath compilation (#2614) fix #2514 main_v11.1 clang (#2628) fix #2644 develop percentile (#2647)
Expected Differences
This is the 2nd pull request for #2476. In TC-Pairs, we need to move the call to
filter_tracks()
down after the call toderive_consensus()
. Otherwise, we discard the consensus member tracks forUEMN_CONS
prior to using them to define the consensus.Do these changes introduce new tools, command line arguments, or configuration file options? [No]
If yes, please describe:
Do these changes modify the structure of existing or add new output data types (e.g. statistic line types or NetCDF variables)? [No]
If yes, please describe:
Pull Request Testing
Describe testing already performed for these changes:
Ran locally to confirm that this fixes the behavior.
Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
Inspect the GHA testing workflow run. The only diffs should be the introduction of a new TC-Pairs output file and also an unrelated issue with the derivation of PBL observations.
Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes]
Already included in earlier PR Feature 2476 tc pairs diag #2694.
Do these changes include sufficient testing updates? [Yes]
Already included in earlier PR Feature 2476 tc pairs diag #2694.
Will this PR result in changes to the test suite? [Yes]
If yes, describe the new output and/or changes to the existing output:
New output file is added:
Expect to see an unrelated PB2NC difference flagged as well:
Pull Request Checklist
See the METplus Workflow for details.
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