Skip to content

Update TestReference for ObsErrorFactorConventional PR - #600

Merged
BenjaminRuston merged 4 commits into
developfrom
bugfix/convLayers
Oct 8, 2026
Merged

BenjaminRuston merged 4 commits into
developfrom
bugfix/convLayers

Conversation

@ClaraDraper-NOAA

@ClaraDraper-NOAA ClaraDraper-NOAA commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Update reference file for one test, after changed in:

build-group=https://github.com/JCSDA-internal/ufo/pull/4391

Two of 36 values change in one test changed. both failures are at low VAD levels, and both get smaller, which is exactly as as expected.

Checklist

  • [x ] I have performed a self-review of my own code
  • [ x] I have made corresponding changes to the documentation
  • [ x] I have run the unit tests before creating the PR

…r fix

Regenerated the reference error factors used by
ufo_function_obserrorfactorconv with the fix in ufo bugfix/convLayers
(ObsErrorFactorConventional now uses the model layer containing the
observation, as GSI errormod). Two values change, both at the lowest
VAD levels and both decrease: 982 hPa 1.08110 -> 1.07982,
968 hPa 1.07835 -> 1.07834. windNorthward and all other variables
are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 1, 2026 18:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The binary NetCDF changes cannot be inspected from the LFS pointer, and the referenced upstream PR was inaccessible.

Review effort: Balanced
Findings: None

What changed in this PR

Updates a VAD wind observation reference dataset following upstream error-factor changes.

Changes:

  • Replaces the Git LFS object for the NetCDF test fixture.
  • Preserves the file size at 111,912 bytes.
File Description
testinput_tier_1/​converr_vadwind_obs_2020120112_s.nc4 Updates the reference dataset’s LFS pointer.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@ClaraDraper-NOAA ClaraDraper-NOAA changed the title Update TestReference/windEastward for ObsErrorFactorConventional laye… Update TestReference for ObsErrorFactorConventional PR Oct 1, 2026
@BenjaminRuston BenjaminRuston added the coordinated merge Needs to be coordinated with other repos label Oct 2, 2026
@BenjaminRuston

Copy link
Copy Markdown
Collaborator

think we need to add the build group here for the test to succeed, will do that still getting

test_obserrorfactorconv: using ObsGroup obs data container
test_obserrorfactorconv: read database from Data/ufo/testinput_tier_1/converr_vadwind_obs_2020120112_s.nc4 (io pool size: 1)
test_obserrorfactorconv processed vars: 2 Variables: windEastward, windNorthward
test_obserrorfactorconv assimilated vars: 1 Variables: windEastward
Vector difference between reference and computed: 

test_obserrorfactorconv windEastward nlocs = 36, nobs = 36, min = 0, max = 0.00128543, avg = 3.57065e-05
Test "ufo/ObsFunction/testFunction" failed: Condition failed: rms_out[ivar] < 100*tol @  (/workdir/bundle/ufo/test/mains/../ufo/ObsFunction.h:133 checkResults)
Completed case 0: ufo/ObsFunction/testFunction
	FAILED: ufo/ObsFunction/testFunction
1 tests failed out of 1.

@ClaraDraper-NOAA

Copy link
Copy Markdown
Contributor Author

think we need to add the build group here for the test to succeed, will do that still getting

test_obserrorfactorconv: using ObsGroup obs data container
test_obserrorfactorconv: read database from Data/ufo/testinput_tier_1/converr_vadwind_obs_2020120112_s.nc4 (io pool size: 1)
test_obserrorfactorconv processed vars: 2 Variables: windEastward, windNorthward
test_obserrorfactorconv assimilated vars: 1 Variables: windEastward
Vector difference between reference and computed: 

test_obserrorfactorconv windEastward nlocs = 36, nobs = 36, min = 0, max = 0.00128543, avg = 3.57065e-05
Test "ufo/ObsFunction/testFunction" failed: Condition failed: rms_out[ivar] < 100*tol @  (/workdir/bundle/ufo/test/mains/../ufo/ObsFunction.h:133 checkResults)
Completed case 0: ufo/ObsFunction/testFunction
	FAILED: ufo/ObsFunction/testFunction
1 tests failed out of 1.

I'm afraid I don't understand this comment. Are you saying that it's failing after you added a build-group? This PR and the have build-group, but maybe I did it wrong?

@BenjaminRuston BenjaminRuston left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the update @ClaraDraper-NOAA

@BenjaminRuston BenjaminRuston added the needs review Asking others to review - often used for pull requests label Oct 7, 2026
@BenjaminRuston
BenjaminRuston requested a review from delippi October 7, 2026 21:39

@fcvdb fcvdb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Clara!

@BenjaminRuston
BenjaminRuston merged commit 55298a5 into develop Oct 8, 2026
2 checks passed
@BenjaminRuston
BenjaminRuston deleted the bugfix/convLayers branch October 8, 2026 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

coordinated merge Needs to be coordinated with other repos needs review Asking others to review - often used for pull requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants