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

Update Truth: Based on dtcenter/MET#2838 #2515

Closed
6 of 9 tasks
JohnHalleyGotway opened this issue Mar 14, 2024 · 1 comment
Closed
6 of 9 tasks

Update Truth: Based on dtcenter/MET#2838 #2515

JohnHalleyGotway opened this issue Mar 14, 2024 · 1 comment
Assignees
Labels
alert: NEED ACCOUNT KEY Need to assign an account key to this issue component: CI/CD Continuous integration and deployment issues priority: blocker Blocker requestor: METplus Team METplus Development Team type: update truth Update truth dataset
Milestone

Comments

@JohnHalleyGotway
Copy link
Collaborator

JohnHalleyGotway commented Mar 14, 2024

Describe Expected Changes

Issue dtcenter/MET#2583 and pull request dtcenter/MET#2838 adds new columns to the end of the ECNT line type.

The changes should be limited to only the ECNT line type.

Need to validate the differences flagged in this GHA testing workflow run.

Define the Metadata

Title

Assignee

Assign this issue to the author of the pull request that warranted this issue. Optionally assign anyone else who should review the differences in the output.

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

Projects and Milestone

  • Select Repository and/or Organization level Project(s) or add alert: NEED CYCLE ASSIGNMENT label
  • Select Milestone as the next official version or Future Versions

Update Truth Checklist

  • Review the GitHub Actions workflow that was triggered by the PR merge
    • If no differences were found, note this in a comment.
    • If all of the differences are expected, note this in a comment.
      Include any details of how the review was performed.
    • If unexpected differences are found, the following instructions can
      help uncover potential explanations. If none of these apply and the
      source of the differences cannot be determined, contact the
      METplus wrappers lead engineer (@georgemccabe) for assistance.
      • Search for other open issues that have the label type: update truth
        applied by clicking on the label on this issue. Coordinate with the
        author of these issues to ensure all diffs are properly reviewed.
      • Check if any additional GitHub Actions testing workflows have been
        triggered since the workflow that corresponds to this issue was run.
        Review the latest run to ensure that there are no diffs that are
        unrelated to this issue.
      • If the incorrect differences are caused by the changes from the
        issue that warranted this issue, consider reverting the PR and
        re-opening the issue.
    • Iterate until one of the above conditions apply.
  • Approve the update of the truth data
    • Contact the METplus wrappers lead engineer (@georgemccabe) or
      backup lead (@jprestop) to let them know that the truth data can
      be updated.
  • Update the truth data.
    This should be handled by a METplus wrappers engineer.
    See the (instructions to update the truth data)[https://metplus.readthedocs.io/en/develop/Contributors_Guide/continuous_integration.html#update-truth-data-update-truth-data-yml]
    for more info.
  • Close this issue.
@JohnHalleyGotway JohnHalleyGotway added priority: blocker Blocker alert: NEED ACCOUNT KEY Need to assign an account key to this issue component: CI/CD Continuous integration and deployment issues requestor: METplus Team METplus Development Team type: update truth Update truth dataset labels Mar 14, 2024
@JohnHalleyGotway JohnHalleyGotway added this to the METplus-6.0.0 milestone Mar 14, 2024
@JohnHalleyGotway
Copy link
Collaborator Author

JohnHalleyGotway commented Mar 14, 2024

Checking this GHA testing workflow run, I see that differences are flagged in 3 use case groups across a total of 11 different output files, all output from the Ensemble-Stat tool.

I ran the following commands to confirm that the only source of difference is the 2 new ECNT columns.

for truth in `find ./ -name "*truth*"`; do 
  output=`echo $truth | sed 's/truth/output/g'`;
  egrep "ECNT|VERSION" $truth | sed -r 's/ +/ /g' > truth_ecnt.txt
  egrep -v "ECNT|VERSION" $truth | sed -r 's/ +/ /g' > truth_not_ecnt.txt
  egrep "ECNT|VERSION" $output | sed -r 's/ +/ /g' | cut -d' ' -f1-49 > output_ecnt.txt
  egrep -v "ECNT|VERSION" $output | sed -r 's/ +/ /g' > output_not_ecnt.txt
  echo "+++ $truth +++"
  diff truth_ecnt.txt output_ecnt.txt
  diff truth_not_ecnt.txt output_not_ecnt.txt
done

With this approach of directly diffing the non-ECNT lines and diffing only the 49 ECNT columns common to both, no differences were found. That means that changes are limited to the newly added columns 50 and 51 of the ECNT line type.

@georgemccabe, the develop-ref truth dataset can safely be updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alert: NEED ACCOUNT KEY Need to assign an account key to this issue component: CI/CD Continuous integration and deployment issues priority: blocker Blocker requestor: METplus Team METplus Development Team type: update truth Update truth dataset
Projects
Status: 🏁 Done
Development

No branches or pull requests

2 participants