Skip to content

[1766][1743][1332] lint and unit-test fix#1802

Merged
tjhunter merged 7 commits intoecmwf:developfrom
simone99n:simone99n/dev/1766_lint_fix
Mar 9, 2026
Merged

[1766][1743][1332] lint and unit-test fix#1802
tjhunter merged 7 commits intoecmwf:developfrom
simone99n:simone99n/dev/1766_lint_fix

Conversation

@simone99n
Copy link
Copy Markdown
Contributor

@simone99n simone99n commented Feb 4, 2026

Description

Issue Number

Checklist before asking for review

  • I have performed a self-review of my code
  • My changes comply with basic sanity checks:
    • I have fixed formatting issues with ./scripts/actions.sh lint
    • I have run unit tests with ./scripts/actions.sh unit-test
    • I have documented my code and I have updated the docstrings.
    • I have added unit tests, if relevant
  • I have tried my changes with data and code:
    • I have run the integration tests with ./scripts/actions.sh integration-test
    • (bigger changes) I have run a full training and I have written in the comment the run_id(s): launch-slurm.py --time 60
    • (bigger changes and experiments) I have shared a hegdedoc in the github issue with all the configurations and runs for this experiments
  • I have informed and aligned with people impacted by my change:
    • for config changes: the MatterMost channels and/or a design doc
    • for changes of dependencies: the MatterMost software development channel

@simone99n simone99n force-pushed the simone99n/dev/1766_lint_fix branch from bee363e to 6e83cf3 Compare February 17, 2026 08:37
@simone99n simone99n changed the title [1766][1742] lint and unit-test fix [1766][1743][1332] lint and unit-test fix Feb 17, 2026
@simone99n simone99n marked this pull request as ready for review February 17, 2026 08:41
@simone99n simone99n force-pushed the simone99n/dev/1766_lint_fix branch from 6e83cf3 to 9d075ae Compare February 18, 2026 07:50
@github-actions github-actions Bot added the bug Something isn't working label Feb 19, 2026
Copy link
Copy Markdown
Collaborator

@tjhunter tjhunter left a comment

Choose a reason for hiding this comment

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

@simone99n thanks for tackling these issues. There is an issue with your logic, let's discuss it.

Comment thread scripts/actions.sh Outdated
ruff check --target-version py312 \
--fix \
src/ scripts/ packages/
echo "LINTING WORKFLOW STARTING..."
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.

this is not correct. lint is here to run the formatting commands, the -check are here to ensure that you have run the commands first. The output for example should not mention a reformatting:

lint-check...
Installed 1 package in 93ms
1 file reformatted, 102 files left unchanged <-- should not reformat

Comment thread scripts/actions.sh Outdated
Comment thread scripts/check_badfunctions.py Outdated
@github-project-automation github-project-automation Bot moved this to In Progress in WeatherGen-dev Feb 23, 2026
Comment thread scripts/actions.sh Outdated
echo "LINTING WORKFLOW STARTING..."

echo "lint-check..."
./scripts/actions.sh lint-check
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.

add && chaining to ensure they all run and fail together

Comment thread scripts/actions.sh Outdated
echo "lint-check..."
./scripts/actions.sh lint-check
echo "lint-fix..."
./scripts/actions.sh lint-fix
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.

it should not be here?

Comment thread scripts/actions.sh Outdated
echo "toml-checking..."
./scripts/actions.sh toml-check
echo "type-checking..."
./scripts/actions.sh type-check
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.

we can leave this one as warning (it should not fail the linting)

@simone99n simone99n force-pushed the simone99n/dev/1766_lint_fix branch from 9d075ae to f945561 Compare February 23, 2026 11:55
@tjhunter tjhunter added the infra Issues related to infrastructure label Feb 27, 2026
@tjhunter
Copy link
Copy Markdown
Collaborator

tjhunter commented Mar 9, 2026

@simone99n let's whitelist these few lines for the time being and fix them later.

@simone99n
Copy link
Copy Markdown
Contributor Author

@tjhunter done.

Copy link
Copy Markdown
Collaborator

@tjhunter tjhunter left a comment

Choose a reason for hiding this comment

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

It is better to put a message such asth #noqa on the lines that are concerned rather than the whole file, because then people can add more problematic code in the meantime in the file. Let's merge for now.

@tjhunter tjhunter merged commit 4906c6e into ecmwf:develop Mar 9, 2026
5 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in WeatherGen-dev Mar 9, 2026
Jubeku added a commit that referenced this pull request Mar 10, 2026
* Improve support for latent losses (#1963)

* Revert 2D rope to false by default (#1967)

Set to True by accident

* Implementation of DataReaderMesh (#1840)

* First implementation of DataReaderMesh

* Move to datareaders extra

* ruff

* ruff2

* Undo ruff

* undo auto-linting

* correct typo in eval config (#1971)

* Added all-physical-streams option and x/y axis limits (#1972)

* Added all-physical-streams option and x/y axis limits

* Fix

* Changed flag for all streams

* Removed old code

* moved metric parsing to eval_from_config (#1977)

Co-authored-by: buschow1 <buschow1@jwlogin04.juwels>

* Fixed integration test (#1980)

* [1974][model] Add fallback to config loading (#1985)

* Add fallback to config loading

* Adjust error message to be not misleading

* Homegenize naming convention

* Introduce bias/diff maps and animations (#1912)

* Introduce bias/diff maps and animations

* minor correction

* Changes based on review

* Introduce "plot_bias" in evaluation configuration (#1986)

* Fixed index ordering to not have shuffled output (#1982)

* Fixed idxs_inv to revert data point shuffeling

* Fixed output handling

* Handling of empty data case, addressing reviewer comment

* [1893][eval] csvreader cleanup (#1906)

* refactor csvreader

* check if dataarray size is 0

* fix and use original logic for empty data

* linting fixes

* revert assertions back

* [1890][eval] Move MergeReader to own module (#1892)

* move mergereader

* use assertions only

* implement scoring for the sub-steps within the forecast window    (#1896)

* work in progress

* working for forecast_step

* working version

* restore no valid times option

* lint

* Rename scale_z_channels to _scale_z_channels

* fix 1 sample bug

* Remove points_per_sample from ReaderOutput

Remove points_per_sample from ReaderOutput return.

* remove n_point_per_sample

* fix lead time coord in compute_scores

* lint

* fix integration test

* Fix integration test single stream (#1996)

* fix test single

* change yml extension and minor fixes

---------

Co-authored-by: cosi1 <cosi1@jwlogin21.juwels>
Co-authored-by: cosi1 <cosi1@jwb0001.juwels>

* [1907][eval] clean up wegen_reader.py (#1911)

* clean up wegen_reader.py

* remove exception

* consistent reader naming

* add blank line

* use assertions only

* make names consistent

* Merge branch 'develop' into 1907-wegenreader-cleanup

* revert is_regular

---------

Co-authored-by: iluise <72020169+iluise@users.noreply.github.com>
Co-authored-by: Ilaria Luise <luise.ilaria@gmail.com>

* [1888][eval] Refactor Reader class (#1889)

* refactor Reader

* use assertion only

* fix npp atms

---------

Co-authored-by: iluise <72020169+iluise@users.noreply.github.com>
Co-authored-by: Ilaria Luise <luise.ilaria@gmail.com>

* [1975][model] Load model path from private repo instead of json (#1998)

* Load model path from private repo instead of json

* Lint

* Script to compute spatial autocorrelation of structured/unstructured datasets (#1955)

* standalone script to compute spatial autocorrelation of variables in a structured or unstructured dataset

* remove commits that should be in pr 1951

* lint

* addressed comments

* removed last failure returning 500km default, and moved to packages science

* updated a note

* rename autocorrelation script

* update example usage

* Correct EMA halflife_steps calculation with rampup_ratio (#2001)

Corrected rampup calculation: https://github.com/NVlabs/edm2/blob/4bf8162f601bcc09472ce8a32dd0cbe8889dc8fc/training/phema.py#L145

Co-authored-by: Wael <wael.almikaeel.95@gmail.com>

* Reduce verbosity of output during inference and evaluation  (#2006)

* Fix incorrect length in validation progress bar

* Removing too verbose output

* [1766][1743][1332] lint and unit-test fix (#1802)

* [1766][1742] fix lint and unit-test

* [1766] fix linter

* [1766] lint local and global consistent

* [1332] add script to detect bad functions (getattr)

* code quality: lint and bad functions

* [1766] disable some checks

* [1877] Script to populate PR labels from linked issues (#1878)

* script

* branch

* more dirs

* typo

* enable

* Fixed bug in linear embedding (#2012)

* Adding forecast_steps feature to plot_train (#2010)

* Adding forecast_steps feature to plot_train

* Renamed arguement to conform to hyphen convention

* Added forecast step to filename

---------

Co-authored-by: Seb Hickman <56727418+shmh40@users.noreply.github.com>

---------

Co-authored-by: Christian Lessig <christian.lessig@ecmwf.int>
Co-authored-by: Seb Hickman <56727418+shmh40@users.noreply.github.com>
Co-authored-by: Kacper Nowak <kacper.nowak@awi.de>
Co-authored-by: Till Hauer <till@web-hauer.de>
Co-authored-by: s6sebusc <49226935+s6sebusc@users.noreply.github.com>
Co-authored-by: buschow1 <buschow1@jwlogin04.juwels>
Co-authored-by: Matthias Karlbauer <matthias.karlbauer@ecmwf.int>
Co-authored-by: Savvas Melidonis <79579567+SavvasMel@users.noreply.github.com>
Co-authored-by: Michael Tarnawa <18899420+mtar@users.noreply.github.com>
Co-authored-by: iluise <72020169+iluise@users.noreply.github.com>
Co-authored-by: pierluigicosi <91318382+pierluigicosi@users.noreply.github.com>
Co-authored-by: cosi1 <cosi1@jwlogin21.juwels>
Co-authored-by: cosi1 <cosi1@jwb0001.juwels>
Co-authored-by: Ilaria Luise <luise.ilaria@gmail.com>
Co-authored-by: Wael <wael.almikaeel.95@gmail.com>
Co-authored-by: Simone Norberti <63310821+simone99n@users.noreply.github.com>
Co-authored-by: Timothy Hunter <tim.hunter@ecmwf.int>
Jubeku added a commit that referenced this pull request Mar 12, 2026
* Improve support for latent losses (#1963)

* Revert 2D rope to false by default (#1967)

Set to True by accident

* Implementation of DataReaderMesh (#1840)

* First implementation of DataReaderMesh

* Move to datareaders extra

* ruff

* ruff2

* Undo ruff

* undo auto-linting

* correct typo in eval config (#1971)

* Added all-physical-streams option and x/y axis limits (#1972)

* Added all-physical-streams option and x/y axis limits

* Fix

* Changed flag for all streams

* Removed old code

* moved metric parsing to eval_from_config (#1977)

Co-authored-by: buschow1 <buschow1@jwlogin04.juwels>

* Fixed integration test (#1980)

* [1974][model] Add fallback to config loading (#1985)

* Add fallback to config loading

* Adjust error message to be not misleading

* Homegenize naming convention

* Introduce bias/diff maps and animations (#1912)

* Introduce bias/diff maps and animations

* minor correction

* Changes based on review

* Introduce "plot_bias" in evaluation configuration (#1986)

* Fixed index ordering to not have shuffled output (#1982)

* Fixed idxs_inv to revert data point shuffeling

* Fixed output handling

* Handling of empty data case, addressing reviewer comment

* [1893][eval] csvreader cleanup (#1906)

* refactor csvreader

* check if dataarray size is 0

* fix and use original logic for empty data

* linting fixes

* revert assertions back

* [1890][eval] Move MergeReader to own module (#1892)

* move mergereader

* use assertions only

* implement scoring for the sub-steps within the forecast window    (#1896)

* work in progress

* working for forecast_step

* working version

* restore no valid times option

* lint

* Rename scale_z_channels to _scale_z_channels

* fix 1 sample bug

* Remove points_per_sample from ReaderOutput

Remove points_per_sample from ReaderOutput return.

* remove n_point_per_sample

* fix lead time coord in compute_scores

* lint

* fix integration test

* Fix integration test single stream (#1996)

* fix test single

* change yml extension and minor fixes

---------

Co-authored-by: cosi1 <cosi1@jwlogin21.juwels>
Co-authored-by: cosi1 <cosi1@jwb0001.juwels>

* [1907][eval] clean up wegen_reader.py (#1911)

* clean up wegen_reader.py

* remove exception

* consistent reader naming

* add blank line

* use assertions only

* make names consistent

* Merge branch 'develop' into 1907-wegenreader-cleanup

* revert is_regular

---------

Co-authored-by: iluise <72020169+iluise@users.noreply.github.com>
Co-authored-by: Ilaria Luise <luise.ilaria@gmail.com>

* [1888][eval] Refactor Reader class (#1889)

* refactor Reader

* use assertion only

* fix npp atms

---------

Co-authored-by: iluise <72020169+iluise@users.noreply.github.com>
Co-authored-by: Ilaria Luise <luise.ilaria@gmail.com>

* [1975][model] Load model path from private repo instead of json (#1998)

* Load model path from private repo instead of json

* Lint

* Script to compute spatial autocorrelation of structured/unstructured datasets (#1955)

* standalone script to compute spatial autocorrelation of variables in a structured or unstructured dataset

* remove commits that should be in pr 1951

* lint

* addressed comments

* removed last failure returning 500km default, and moved to packages science

* updated a note

* rename autocorrelation script

* update example usage

* Correct EMA halflife_steps calculation with rampup_ratio (#2001)

Corrected rampup calculation: https://github.com/NVlabs/edm2/blob/4bf8162f601bcc09472ce8a32dd0cbe8889dc8fc/training/phema.py#L145

Co-authored-by: Wael <wael.almikaeel.95@gmail.com>

* Reduce verbosity of output during inference and evaluation  (#2006)

* Fix incorrect length in validation progress bar

* Removing too verbose output

* [1766][1743][1332] lint and unit-test fix (#1802)

* [1766][1742] fix lint and unit-test

* [1766] fix linter

* [1766] lint local and global consistent

* [1332] add script to detect bad functions (getattr)

* code quality: lint and bad functions

* [1766] disable some checks

* [1877] Script to populate PR labels from linked issues (#1878)

* script

* branch

* more dirs

* typo

* enable

* Fixed bug in linear embedding (#2012)

* Adding forecast_steps feature to plot_train (#2010)

* Adding forecast_steps feature to plot_train

* Renamed arguement to conform to hyphen convention

* Added forecast step to filename

---------

Co-authored-by: Seb Hickman <56727418+shmh40@users.noreply.github.com>

* add noise distribution plotting

* plot noise distribution and decoded noised tokens

* fix noise level in validation to p_mean

* rm noise and token distribution plotting

---------

Co-authored-by: Christian Lessig <christian.lessig@ecmwf.int>
Co-authored-by: Seb Hickman <56727418+shmh40@users.noreply.github.com>
Co-authored-by: Kacper Nowak <kacper.nowak@awi.de>
Co-authored-by: Till Hauer <till@web-hauer.de>
Co-authored-by: s6sebusc <49226935+s6sebusc@users.noreply.github.com>
Co-authored-by: buschow1 <buschow1@jwlogin04.juwels>
Co-authored-by: Matthias Karlbauer <matthias.karlbauer@ecmwf.int>
Co-authored-by: Savvas Melidonis <79579567+SavvasMel@users.noreply.github.com>
Co-authored-by: Michael Tarnawa <18899420+mtar@users.noreply.github.com>
Co-authored-by: iluise <72020169+iluise@users.noreply.github.com>
Co-authored-by: pierluigicosi <91318382+pierluigicosi@users.noreply.github.com>
Co-authored-by: cosi1 <cosi1@jwlogin21.juwels>
Co-authored-by: cosi1 <cosi1@jwb0001.juwels>
Co-authored-by: Ilaria Luise <luise.ilaria@gmail.com>
Co-authored-by: Wael <wael.almikaeel.95@gmail.com>
Co-authored-by: Simone Norberti <63310821+simone99n@users.noreply.github.com>
Co-authored-by: Timothy Hunter <tim.hunter@ecmwf.int>
Jubeku added a commit that referenced this pull request Mar 15, 2026
* grad accumulation first try

* better memory

* revert grad accumulation

* Jk/mk/mh/diffusion single sample fix val (#2045)

* Improve support for latent losses (#1963)

* Revert 2D rope to false by default (#1967)

Set to True by accident

* Implementation of DataReaderMesh (#1840)

* First implementation of DataReaderMesh

* Move to datareaders extra

* ruff

* ruff2

* Undo ruff

* undo auto-linting

* correct typo in eval config (#1971)

* Added all-physical-streams option and x/y axis limits (#1972)

* Added all-physical-streams option and x/y axis limits

* Fix

* Changed flag for all streams

* Removed old code

* moved metric parsing to eval_from_config (#1977)

Co-authored-by: buschow1 <buschow1@jwlogin04.juwels>

* Fixed integration test (#1980)

* [1974][model] Add fallback to config loading (#1985)

* Add fallback to config loading

* Adjust error message to be not misleading

* Homegenize naming convention

* Introduce bias/diff maps and animations (#1912)

* Introduce bias/diff maps and animations

* minor correction

* Changes based on review

* Introduce "plot_bias" in evaluation configuration (#1986)

* Fixed index ordering to not have shuffled output (#1982)

* Fixed idxs_inv to revert data point shuffeling

* Fixed output handling

* Handling of empty data case, addressing reviewer comment

* [1893][eval] csvreader cleanup (#1906)

* refactor csvreader

* check if dataarray size is 0

* fix and use original logic for empty data

* linting fixes

* revert assertions back

* [1890][eval] Move MergeReader to own module (#1892)

* move mergereader

* use assertions only

* implement scoring for the sub-steps within the forecast window    (#1896)

* work in progress

* working for forecast_step

* working version

* restore no valid times option

* lint

* Rename scale_z_channels to _scale_z_channels

* fix 1 sample bug

* Remove points_per_sample from ReaderOutput

Remove points_per_sample from ReaderOutput return.

* remove n_point_per_sample

* fix lead time coord in compute_scores

* lint

* fix integration test

* Fix integration test single stream (#1996)

* fix test single

* change yml extension and minor fixes

---------

Co-authored-by: cosi1 <cosi1@jwlogin21.juwels>
Co-authored-by: cosi1 <cosi1@jwb0001.juwels>

* [1907][eval] clean up wegen_reader.py (#1911)

* clean up wegen_reader.py

* remove exception

* consistent reader naming

* add blank line

* use assertions only

* make names consistent

* Merge branch 'develop' into 1907-wegenreader-cleanup

* revert is_regular

---------

Co-authored-by: iluise <72020169+iluise@users.noreply.github.com>
Co-authored-by: Ilaria Luise <luise.ilaria@gmail.com>

* [1888][eval] Refactor Reader class (#1889)

* refactor Reader

* use assertion only

* fix npp atms

---------

Co-authored-by: iluise <72020169+iluise@users.noreply.github.com>
Co-authored-by: Ilaria Luise <luise.ilaria@gmail.com>

* [1975][model] Load model path from private repo instead of json (#1998)

* Load model path from private repo instead of json

* Lint

* Script to compute spatial autocorrelation of structured/unstructured datasets (#1955)

* standalone script to compute spatial autocorrelation of variables in a structured or unstructured dataset

* remove commits that should be in pr 1951

* lint

* addressed comments

* removed last failure returning 500km default, and moved to packages science

* updated a note

* rename autocorrelation script

* update example usage

* Correct EMA halflife_steps calculation with rampup_ratio (#2001)

Corrected rampup calculation: https://github.com/NVlabs/edm2/blob/4bf8162f601bcc09472ce8a32dd0cbe8889dc8fc/training/phema.py#L145

Co-authored-by: Wael <wael.almikaeel.95@gmail.com>

* Reduce verbosity of output during inference and evaluation  (#2006)

* Fix incorrect length in validation progress bar

* Removing too verbose output

* [1766][1743][1332] lint and unit-test fix (#1802)

* [1766][1742] fix lint and unit-test

* [1766] fix linter

* [1766] lint local and global consistent

* [1332] add script to detect bad functions (getattr)

* code quality: lint and bad functions

* [1766] disable some checks

* [1877] Script to populate PR labels from linked issues (#1878)

* script

* branch

* more dirs

* typo

* enable

* Fixed bug in linear embedding (#2012)

* Adding forecast_steps feature to plot_train (#2010)

* Adding forecast_steps feature to plot_train

* Renamed arguement to conform to hyphen convention

* Added forecast step to filename

---------

Co-authored-by: Seb Hickman <56727418+shmh40@users.noreply.github.com>

* add noise distribution plotting

* plot noise distribution and decoded noised tokens

* fix noise level in validation to p_mean

* rm noise and token distribution plotting

---------

Co-authored-by: Christian Lessig <christian.lessig@ecmwf.int>
Co-authored-by: Seb Hickman <56727418+shmh40@users.noreply.github.com>
Co-authored-by: Kacper Nowak <kacper.nowak@awi.de>
Co-authored-by: Till Hauer <till@web-hauer.de>
Co-authored-by: s6sebusc <49226935+s6sebusc@users.noreply.github.com>
Co-authored-by: buschow1 <buschow1@jwlogin04.juwels>
Co-authored-by: Matthias Karlbauer <matthias.karlbauer@ecmwf.int>
Co-authored-by: Savvas Melidonis <79579567+SavvasMel@users.noreply.github.com>
Co-authored-by: Michael Tarnawa <18899420+mtar@users.noreply.github.com>
Co-authored-by: iluise <72020169+iluise@users.noreply.github.com>
Co-authored-by: pierluigicosi <91318382+pierluigicosi@users.noreply.github.com>
Co-authored-by: cosi1 <cosi1@jwlogin21.juwels>
Co-authored-by: cosi1 <cosi1@jwb0001.juwels>
Co-authored-by: Ilaria Luise <luise.ilaria@gmail.com>
Co-authored-by: Wael <wael.almikaeel.95@gmail.com>
Co-authored-by: Simone Norberti <63310821+simone99n@users.noreply.github.com>
Co-authored-by: Timothy Hunter <tim.hunter@ecmwf.int>

* implement zero-weight physical loss

* plot config file

* deletion

* merged with noised plotting

* revert some changes

* revert change for fixed eta

* revert changes

* revert more changes

* fixed noised plotting

* config changes

---------

Co-authored-by: Julian Kuehnert <Jubeku@users.noreply.github.com>
Co-authored-by: Christian Lessig <christian.lessig@ecmwf.int>
Co-authored-by: Seb Hickman <56727418+shmh40@users.noreply.github.com>
Co-authored-by: Kacper Nowak <kacper.nowak@awi.de>
Co-authored-by: Till Hauer <till@web-hauer.de>
Co-authored-by: s6sebusc <49226935+s6sebusc@users.noreply.github.com>
Co-authored-by: buschow1 <buschow1@jwlogin04.juwels>
Co-authored-by: Matthias Karlbauer <matthias.karlbauer@ecmwf.int>
Co-authored-by: Savvas Melidonis <79579567+SavvasMel@users.noreply.github.com>
Co-authored-by: Michael Tarnawa <18899420+mtar@users.noreply.github.com>
Co-authored-by: iluise <72020169+iluise@users.noreply.github.com>
Co-authored-by: pierluigicosi <91318382+pierluigicosi@users.noreply.github.com>
Co-authored-by: cosi1 <cosi1@jwlogin21.juwels>
Co-authored-by: cosi1 <cosi1@jwb0001.juwels>
Co-authored-by: Ilaria Luise <luise.ilaria@gmail.com>
Co-authored-by: Wael <wael.almikaeel.95@gmail.com>
Co-authored-by: Simone Norberti <63310821+simone99n@users.noreply.github.com>
Co-authored-by: Timothy Hunter <tim.hunter@ecmwf.int>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working infra Issues related to infrastructure

Projects

Status: Done

2 participants