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: For dtcenter/MET#3007 #2873

Closed
10 of 11 tasks
JohnHalleyGotway opened this issue Jan 24, 2025 · 1 comment · Fixed by #2874
Closed
10 of 11 tasks

Update Truth: For dtcenter/MET#3007 #2873

JohnHalleyGotway opened this issue Jan 24, 2025 · 1 comment · Fixed by #2874
Assignees
Labels
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 Jan 24, 2025

Describe Expected Changes

Write a short summary of the differences that will likely be found in the GitHub Actions testing workflow that was triggered by the pull request that warranted this issue

Define the Metadata

Title

  • Define the Title of this issue as Update Truth: For dtcenter/{REPO}#{PR_NUMBER} to indicate the repository and pull request that warranted this issue.

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

Milestone and Projects

  • Select Milestone as the next official version if updating truth data for the develop branch OR select next METplus-Wrappers-X.Y.Z bugfix version if updating truth data for a main_vX.Y branch.
  • If updating truth data for the develop branch, select the METplus-Wrappers-X.Y.Z Development project OR if updating truth for a main_vX.Y branch, select the Coordinated METplus-X.Y Support project

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
    for more info.
  • Close this issue.
@JohnHalleyGotway JohnHalleyGotway added component: CI/CD Continuous integration and deployment issues priority: blocker Blocker requestor: METplus Team METplus Development Team type: update truth Update truth dataset labels Jan 24, 2025
@JohnHalleyGotway JohnHalleyGotway added this to the METplus-6.1.0 milestone Jan 24, 2025
@JohnHalleyGotway JohnHalleyGotway self-assigned this Jan 24, 2025
@github-project-automation github-project-automation bot moved this to 🩺 Needs Triage in METplus-6.1.0 Development Jan 24, 2025
@JohnHalleyGotway
Copy link
Collaborator Author

Merging the dtcenter/MET#3057 pull request into develop triggered this METplus GHA testing workflow run. That run flagged differences in one of the existing METplus Use Cases:

 COMPARING marine_and_cryosphere/PointStat_fcstRTOFS_obsARGO_climoWOA23_temp/stats/rtofs.20230318/point_stat_RTOFS_ARGO_temp_Z50_240000L_20230318_000000V.stat
  file_A: /data/truth/marine_and_cryosphere/PointStat_fcstRTOFS_obsARGO_climoWOA23_temp/stats/rtofs.20230318/point_stat_RTOFS_ARGO_temp_Z50_240000L_20230318_000000V.stat
  file_B: /data/output/marine_and_cryosphere/PointStat_fcstRTOFS_obsARGO_climoWOA23_temp/stats/rtofs.20230318/point_stat_RTOFS_ARGO_temp_Z50_240000L_20230318_000000V.stat

The differences in the CNT and SAL1L2 are due to modified climatology inputs.
I ran the following plot_data_plane command to investigate, using versions of it before and after merging dtcenter/MET#3057:

plot_data_plane \
woa23/woa23_decav91C0_t03_04.nc \
woa23_decav91C0_t03_04_BEFORE.ps \
'name = "t_an"; level = "(0,@50,*,*)";' \
-v 4 -log run_pdp_BEFORE.log

Here's the relevant diff from the log files:

< DEBUG 4: NcCfFile::getData() -> setting the unset init time to the valid time of 20050315_000000.
---
> DEBUG 4: NcCfFile::getData() -> setting the unset init time to the valid time of 20050316_000000.
34c34
< DEBUG 4:    valid time: 20050315_000000
---
> DEBUG 4:    valid time: 20050316_000000
36c36
< DEBUG 4:    init time: 20050315_000000
---
> DEBUG 4:    init time: 20050316_000000

The valid time parsed from this file has changed from 20050315_000000 to 20050316_000000. And that change in valid time slightly changes the results of the time interpolation (POINT_STAT_CLIMO_MEAN_TIME_INTERP_METHOD = DW_MEAN) method selected in the METplus conf file.

Inspecting the input file with ncdump -v time I see:

netcdf woa23_decav91C0_t04_04 {
dimensions:
	time = UNLIMITED ; // (1 currently)
...
variables:
	float time(time) ;
		time:standard_name = "time" ;
		time:long_name = "time" ;
		time:climatology = "climatology_bnds" ;
		time:units = "months since 1991-01-01 00:00:00" ;
		time:calendar = "standard" ;
		time:axis = "T" ;
...
data:

 time = 171.4683 ;

And this slight change in the result of using "monthly" time step is consistent with the recent enhancements found in is_leap_year.cc.

@JohnHalleyGotway JohnHalleyGotway moved this from 🩺 Needs Triage to 🏗 In progress in METplus-6.1.0 Development Jan 24, 2025
@JohnHalleyGotway JohnHalleyGotway linked a pull request Jan 24, 2025 that will close this issue
@github-project-automation github-project-automation bot moved this from 🏗 In progress to 🏁 Done in METplus-6.1.0 Development Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

Successfully merging a pull request may close this issue.

1 participant