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

Bugfix: Fix the Point-Stat CNT header line typo causing duplicate SI_BCL column names #2730

Closed
9 of 24 tasks
John-Sharples opened this issue Nov 9, 2023 · 1 comment · Fixed by #2732 or #2733
Closed
9 of 24 tasks
Assignees
Labels
MET: Library Code priority: medium Medium Priority requestor: Australian BOM Australian Bureau of Meteorology type: bug Fix something that is not working

Comments

@John-Sharples
Copy link

John-Sharples commented Nov 9, 2023

Describe the Problem

Small issue one of the devs here at BoM noticed:

point_stat cnt file has a repeated header: Last 2 header values are "SI_BCL", but last one should be "SI_BCU" instead, as per docs here: https://met.readthedocs.io/en/latest/Users_Guide/point-stat.html#id11

Expected Behavior

Environment

To Reproduce

sudo docker run -it dtcenter/metplus:6.0.0-beta1 /bin/bash

pushd /met/MET-develop/scripts

export TEST_OUT_DIR=/metplus/OUTPUT

mkdir -p ${TEST_OUT_DIR}/point_stat

point_stat \
   ../data/sample_fcst/2007033000/nam.t00z.awip1236.tm00.20070330.grb \
   ${TEST_OUT_DIR}/pb2nc/sample_pb.nc \
   config/PointStatConfig \
   -point_obs ${TEST_OUT_DIR}/ascii2nc/sample_ascii.nc \
   -outdir ${TEST_OUT_DIR}/point_stat -v 2

cat ${TEST_OUT_DIR}/point_stat/point_stat_360000L_20070331_120000V_cnt.txt

Relevant Deadlines

NONE

Funding Source

NONE

Define the Metadata

Assignee

  • Select engineer(s) or no engineer required
  • Select scientist(s) or no scientist required

Labels

  • Review default alert labels
  • Select component(s)
  • Select priority
  • Select requestor(s)

Milestone and Projects

  • Select Milestone as the next bugfix version
  • Select Coordinated METplus-X.Y Support project for support of the current coordinated release
  • Select MET-X.Y.Z Development project for development toward the next official release

Define Related Issue(s)

Consider the impact to the other METplus components.

Bugfix Checklist

See the METplus Workflow for details.

  • Complete the issue definition above, including the Time Estimate and Funding Source.
  • Fork this repository or create a branch of main_<Version>.
    Branch name: bugfix_<Issue Number>_main_<Version>_<Description>
  • Fix the bug and test your changes.
  • Add/update log messages for easier debugging.
  • Add/update unit tests.
  • Add/update documentation.
  • Push local changes to GitHub.
  • Submit a pull request to merge into main_<Version>.
    Pull request: bugfix <Issue Number> main_<Version> <Description>
  • Define the pull request metadata, as permissions allow.
    Select: Reviewer(s) and Development issue
    Select: Milestone as the next bugfix version
    Select: Coordinated METplus-X.Y Support project for support of the current coordinated release
  • Iterate until the reviewer(s) accept and merge your changes.
  • Delete your fork or branch.
  • Complete the steps above to fix the bug on the develop branch.
    Branch name: bugfix_<Issue Number>_develop_<Description>
    Pull request: bugfix <Issue Number> develop <Description>
    Select: Reviewer(s) and Development issue
    Select: Milestone as the next official version
    Select: MET-X.Y.Z Development project for development toward the next official release
  • Close this issue.
@John-Sharples John-Sharples added alert: NEED ACCOUNT KEY Need to assign an account key to this issue alert: NEED CYCLE ASSIGNMENT Need to assign to a release development cycle alert: NEED MORE DEFINITION Not yet actionable, additional definition required type: bug Fix something that is not working labels Nov 9, 2023
@JohnHalleyGotway JohnHalleyGotway self-assigned this Nov 9, 2023
@JohnHalleyGotway JohnHalleyGotway added priority: medium Medium Priority MET: Library Code and removed alert: NEED MORE DEFINITION Not yet actionable, additional definition required alert: NEED ACCOUNT KEY Need to assign an account key to this issue alert: NEED CYCLE ASSIGNMENT Need to assign to a release development cycle labels Nov 9, 2023
JohnHalleyGotway added a commit that referenced this issue Nov 10, 2023
…line type. This column name appears in (1) data/table_files/met_header_columns*, (2) internal/test_unit/hdr/met*, (3) the User's Guide, (4) series_analysis.cc, (5) stat_columns.cc, and (6) stat_column_defs.h. I checked all those spots and only found the typo in the last one. It's a one line fix.
@JohnHalleyGotway JohnHalleyGotway linked a pull request Nov 10, 2023 that will close this issue
15 tasks
@JohnHalleyGotway JohnHalleyGotway linked a pull request Nov 10, 2023 that will close this issue
@JohnHalleyGotway
Copy link
Collaborator

Thanks @John-Sharples for finding this bug. It's now fixed in the main_v11.1 and develop branches and will be included in all future releases.

JohnHalleyGotway pushed a commit that referenced this issue Nov 12, 2023
Co-authored-by: jprestop <[email protected]>
Co-authored-by: Seth Linden <[email protected]>
Co-authored-by: John Halley Gotway <[email protected]>
Co-authored-by: Daniel Adriaansen <[email protected]>
Co-authored-by: John and Cindy <[email protected]>
Co-authored-by: rgbullock <[email protected]>
Co-authored-by: Randy Bullock <[email protected]>
Co-authored-by: Dave Albo <[email protected]>
Co-authored-by: Howard Soh <[email protected]>
Co-authored-by: George McCabe <[email protected]>
Co-authored-by: hsoh-u <[email protected]>
Co-authored-by: MET Tools Test Account <[email protected]>
Co-authored-by: Seth Linden <[email protected]>
Co-authored-by: lisagoodrich <[email protected]>
Co-authored-by: davidalbo <[email protected]>
Co-authored-by: Lisa Goodrich <[email protected]>
Co-authored-by: metplus-bot <[email protected]>
Co-authored-by: j-opatz <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Jonathan Vigh <[email protected]>
Co-authored-by: Tracy Hertneky <[email protected]>
Co-authored-by: David Albo <[email protected]>
Co-authored-by: Dan Adriaansen <[email protected]>
Co-authored-by: Julie Prestopnik <[email protected]>
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)
fix #2730 develop SI_BCU (#2732)
@JohnHalleyGotway JohnHalleyGotway changed the title Bugfix: point_stat cnt file header has duplicate value "SI_BCL" Bugfix: Fix the Point-Stat CNT header line typo causing duplicate SI_BCL column names Nov 17, 2023
@JohnHalleyGotway JohnHalleyGotway added the requestor: Australian BOM Australian Bureau of Meteorology label Nov 17, 2023
JohnHalleyGotway pushed a commit that referenced this issue Nov 17, 2023
Co-authored-by: jprestop <[email protected]>
Co-authored-by: Seth Linden <[email protected]>
Co-authored-by: John Halley Gotway <[email protected]>
Co-authored-by: Daniel Adriaansen <[email protected]>
Co-authored-by: John and Cindy <[email protected]>
Co-authored-by: rgbullock <[email protected]>
Co-authored-by: Randy Bullock <[email protected]>
Co-authored-by: Dave Albo <[email protected]>
Co-authored-by: Howard Soh <[email protected]>
Co-authored-by: George McCabe <[email protected]>
Co-authored-by: hsoh-u <[email protected]>
Co-authored-by: MET Tools Test Account <[email protected]>
Co-authored-by: Seth Linden <[email protected]>
Co-authored-by: lisagoodrich <[email protected]>
Co-authored-by: davidalbo <[email protected]>
Co-authored-by: Lisa Goodrich <[email protected]>
Co-authored-by: metplus-bot <[email protected]>
Co-authored-by: j-opatz <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Jonathan Vigh <[email protected]>
Co-authored-by: Tracy Hertneky <[email protected]>
Co-authored-by: David Albo <[email protected]>
Co-authored-by: Dan Adriaansen <[email protected]>
Co-authored-by: Julie Prestopnik <[email protected]>
Co-authored-by: root <root@83062d57c5dd>
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)
fix #2730 develop SI_BCU (#2732)
cgrudz added a commit to CW3E/MET-tools that referenced this issue Oct 3, 2024
Currently testing a version of the method used as a callable utility
with the parallelism running through Cylc a la the GridStat call itself.
This reduces several arguments simplifying the method and making a cleaner
method fit for single purpose.  Refactoring has led me to discovering a
bug in MET dtcenter/MET#2730 that also exists
within GridStat.  Need to roll forward MET to 11.1.0 from 11.0.1 to check
if this is also fixed in GridStat or report bug.  Will resume testing
with MET rolled forward.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MET: Library Code priority: medium Medium Priority requestor: Australian BOM Australian Bureau of Meteorology type: bug Fix something that is not working
Projects
Status: 🏁 Done
Development

Successfully merging a pull request may close this issue.

2 participants