Fix integration test single stream#1996
Merged
clessig merged 2 commits intoecmwf:developfrom Mar 6, 2026
Merged
Conversation
Collaborator
|
@pierluigicosi : Does the test pass the threshold for you? |
Contributor
Author
I raised the threshold to 0.5 but i see sometimes it's not enough. Should we use an higher value? |
clessig
reviewed
Mar 5, 2026
Collaborator
clessig
left a comment
clessig
left a comment
There was a problem hiding this comment.
Minor adjustments still needed
| @@ -1,30 +1,219 @@ | |||
| streams_directory: "./integration_tests/streams/" | |||
Collaborator
There was a problem hiding this comment.
The extension of the file should be yml
integration_tests/small1_test.py
Outdated
| [ | ||
| "inference", | ||
| "train", | ||
| f"--config={WEATHERGEN_HOME}/integration_tests/small1.yaml", |
Collaborator
There was a problem hiding this comment.
This should be --base-config
integration_tests/small1.yaml
Outdated
| run_id: ??? | ||
| run_history: [] | ||
|
|
||
| train_log_freq: |
Collaborator
There was a problem hiding this comment.
This must be train_logging
integration_tests/small1.yaml
Outdated
| training_mode: ["masking"] | ||
|
|
||
| num_mini_epochs: 1 | ||
| samples_per_mini_epoch: 32 #128 #256 |
Collaborator
There was a problem hiding this comment.
Please set to 48 and remove commented out values
integration_tests/small1_test.py
Outdated
| # Check that the loss does not explode in a single mini_epoch | ||
| # This is meant to be a quick test, not a convergence test | ||
| target = 0.25 | ||
| target = 0.5 |
Collaborator
There was a problem hiding this comment.
Please change back to 0.25.
| assert loss_metric is not None, f"'{loss_avg_name}' metric is missing in metrics file" | ||
| # Check that the loss does not explode in a single mini_epoch | ||
| # This is meant to be a quick test, not a convergence test | ||
| assert loss_metric < 0.25, f"'{loss_avg_name}' is {loss_metric}, expected to be below 0.25" |
Collaborator
|
Please implement the other changes that I suggested, in particular base-config. Then we could even lower it
…________________________________
From: pierluigicosi ***@***.***>
Sent: Thursday, March 5, 2026 5:05:48 PM
To: ecmwf/WeatherGenerator ***@***.***>
Cc: Christian Lessig ***@***.***>; Comment ***@***.***>
Subject: Re: [ecmwf/WeatherGenerator] Fix integration test single stream (PR #1996)
[https://avatars.githubusercontent.com/u/91318382?s=20&v=4]pierluigicosi left a comment (ecmwf/WeatherGenerator#1996)<#1996 (comment)>
@pierluigicosi<https://github.com/pierluigicosi> : Does the test pass the threshold for you?
I raised the threshold to 0.5 but i see sometimes it's not enough. Should we use an higher value?
—
Reply to this email directly, view it on GitHub<#1996 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AHCHOHSQA247P5ZYYI4XKID4PGQVZAVCNFSM6AAAAACWILR4J2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DAMBWGEZTMNJQGE>.
You are receiving this because you commented.Message ID: ***@***.***>
|
clessig
approved these changes
Mar 6, 2026
Collaborator
clessig
left a comment
clessig
left a comment
There was a problem hiding this comment.
Thanks for fixing this, much needed.
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fix integration test for single stream:
small1_test.pysmall1.ymlReference to #1860
Issue Number
Closes #1993
Is this PR a draft? Mark it as draft.
Checklist before asking for review
./scripts/actions.sh lint./scripts/actions.sh unit-test./scripts/actions.sh integration-testlaunch-slurm.py --time 60