fix!: Rework Loss Scalings to provide better modularity #52

sahahner · 2024-12-27T13:35:33Z

Solve the problem explained in issue #7 by refactoring the variable scalings into a general variable scaling and a pressure level scaling.
@mc4117 , @pinnstorm and me came up with a new structure. This PR implements this.

Changes by this PR

collect all available loss scalings in anemoi/training/losses/scaling
- the new modular structure makes it easier to implement new scalings
move configuration of all available scalings into config/training/scalers
- we provide a list of possible scalers
- list the scalings in a dictionary you want to apply as scalers in the training_loss configuration
  -> in comparison to before, no changes to the code are necessary to apply additional scalings, but can simply be added to the training-config files
- the new default config files do not change the default scalers to be applied
new features of this PR include
- define new scalers that are applied to a group of model/pressure level variables
- the grouping of variables into surface and pressure level variables is no longer defined by the parsing of strings but is defined in config/training/scalers
- introduce a util function to retrieve this grouping at other places in the code
- tendency scaler:
  - introduce the ability to scale the losses by the statistical tendencies in the dataset. At infinite precision, this is equivalent to training towards a tendency loss
rename loss scalars to scalers, as this is the correct naming for the feature

While this PR does introduce a breaking change to the training-config files, it comes with neat new features, that should be of use to many of us.
We did our best to document the changes to the training-config files as best as possible.

Tasklist

allow several variable level scaling (i.e. pressure level and model level)
implement/update tests
decide: do we want to allow scaling by variable_ref and variable_name, i.e. scale q_50 by q and q_50?
get variable level and name from dataset metadata if available
Change of name: loss scalars to scalers.
move node weights into new scaling submodule

📚 Documentation preview 📚: https://anemoi-training--52.org.readthedocs.build/en/52/

📚 Documentation preview 📚: https://anemoi-graphs--52.org.readthedocs.build/en/52/

📚 Documentation preview 📚: https://anemoi-models--52.org.readthedocs.build/en/52/

b8raoult · 2024-12-30T09:35:33Z

Please consider using the knowledge about variables that come from the dataset metadata. See https://github.com/ecmwf/anemoi-transform/blob/7cbf5f3d4baa37453022a5a97e17cc71a5b8ceeb/src/anemoi/transform/variables/__init__.py#L47

sahahner · 2024-12-30T09:51:50Z

Please consider using the knowledge about variables that come from the dataset metadata. See https://github.com/ecmwf/anemoi-transform/blob/7cbf5f3d4baa37453022a5a97e17cc71a5b8ceeb/src/anemoi/transform/variables/__init__.py#L47

We have given this some thought, and after wanting to use the information from the dataset in the beginning, I have opted for allowing the definition of our own groups here to use different scaling for self-defined groups.
Also, I was also told that it is possible to build datasets without information about the variable types and therefore not to rely on that metadata.
If you have strong opinions on this I am happy to discuss it again.

training/src/anemoi/training/train/forecaster.py

training/src/anemoi/training/train/scaling.py

training/src/anemoi/training/train/forecaster.py

…umstances' of https://github.com/ecmwf/anemoi-core into 7-pressure-level-scalings-only-applied-in-specific-circumstances

FussyDuck · 2025-01-02T11:48:17Z

All committers have signed the CLA.

training/src/anemoi/training/train/scaling.py

JPXKQX · 2025-01-08T11:43:58Z

Hi, I would like to know what you think about making all scalers explicit in the config file. Something similar to the additional_scalers: field, but including not only the scalers per variable, but also the node_loss_weight,... The positive aspect I see is that there would be more homogeneity in the scalers defined in the metrics/loss fields.

mc4117 · 2025-01-09T14:24:20Z

Hi, I would like to know what you think about making all scalers explicit in the config file. Something similar to the additional_scalers: field, but including not only the scalers per variable, but also the node_loss_weight,... The positive aspect I see is that there would be more homogeneity in the scalers defined in the metrics/loss fields.

Seems like a good idea! Would you like to add this in this PR?

JPXKQX · 2025-01-09T14:44:22Z

Hi, I would like to know what you think about making all scalers explicit in the config file. Something similar to the additional_scalers: field, but including not only the scalers per variable, but also the node_loss_weight,... The positive aspect I see is that there would be more homogeneity in the scalers defined in the metrics/loss fields.

Seems like a good idea! Would you like to add this in this PR?

I’m not sure what the best approach is. On the one hand, adding more work to this PR would increase its complexity, which might make it more logical to address this refactor in a future PR. On the other hand, this PR already introduces some changes to the configs, and the future PRs would also involve changes to the configs. From this, it might be better to have 1 PR and communicate all the changes to users at once. What do you think?

pinnstorm · 2025-01-10T11:16:32Z

Hi, I would like to know what you think about making all scalers explicit in the config file. Something similar to the additional_scalers: field, but including not only the scalers per variable, but also the node_loss_weight,... The positive aspect I see is that there would be more homogeneity in the scalers defined in the metrics/loss fields.

Seems like a good idea! Would you like to add this in this PR?

I’m not sure what the best approach is. On the one hand, adding more work to this PR would increase its complexity, which might make it more logical to address this refactor in a future PR. On the other hand, this PR already introduces some changes to the configs, and the future PRs would also involve changes to the configs. From this, it might be better to have 1 PR and communicate all the changes to users at once. What do you think?

I'm happy for it to be included in this PR! Not sure if @sahahner or @mc4117 have other views?

…umstances' of github.com:ecmwf/anemoi-core into 7-pressure-level-scalings-only-applied-in-specific-circumstances

…ell.

…umstances' of https://github.com/ecmwf/anemoi-core into 7-pressure-level-scalings-only-applied-in-specific-circumstances

jakob-schloer · 2025-01-24T13:59:17Z

Hi, I would like to know what you think about making all scalers explicit in the config file. Something similar to the additional_scalers: field, but including not only the scalers per variable, but also the node_loss_weight,... The positive aspect I see is that there would be more homogeneity in the scalers defined in the metrics/loss fields.

Seems like a good idea! Would you like to add this in this PR?

I’m not sure what the best approach is. On the one hand, adding more work to this PR would increase its complexity, which might make it more logical to address this refactor in a future PR. On the other hand, this PR already introduces some changes to the configs, and the future PRs would also involve changes to the configs. From this, it might be better to have 1 PR and communicate all the changes to users at once. What do you think?

I fully agree with @JPXKQX. I personally think that all config keywords related to the loss and its scaling are a bit scattered in the training.
I would suggest bringing the restructuring into this PR. If this PR goes in, it will break old configs, and the restructuring will again break old configs.

Ideally, I think the config could look something like that:

loss_scaling:
	default: 1
	groups:
		default: sfc
		pl: []
	scalers:
        - _target_: anemoi.training.losses.scaling.ConstVariableScaler
              default: 1
	      variables:
	            q: 0.6 #1
		     t: 6   #1
		     u: 0.8 #0.5
                    ...
	- _target_: anemoi.training.losses.scaling.ReluVariableLevelScaler
              group: pl
              y_intercept: 0.2
              ...
	- target_: anemoi.training.losses.nodeweights.GraphNodeAttribute
	      target_nodes: ${graph.data}
             node_attribute: area_weight

jakob-schloer

Great effort! The flexible scaling of the loss is really nice.

However, in its current version all config keywords related to the loss and its scaling as well as the code is a bit scattered. Why is variable scaling under training while nodeweights is under losses? In my opinion everything should be under losses. See my comment above.

training/src/anemoi/training/config/training/default.yaml

training/src/anemoi/training/train/forecaster.py

training/src/anemoi/training/train/scaling.py

…name all scalars to scalers. move nan-mask for loss function into training/scaling

for more information, see https://pre-commit.ci

training/src/anemoi/training/scaling/__init__.py

HCookie · 2025-01-27T17:34:05Z

training/src/anemoi/training/train/forecaster.py

@@ -111,25 +134,24 @@ def __init__(

        # Kwargs to pass to the loss function


This should be added to the new structure.

HCookie · 2025-01-27T17:34:46Z

training/src/anemoi/training/config/training/scalers/scalers.yaml

+  node_weights:
+    _target_: anemoi.training.losses.nodeweights.GraphNodeAttribute
+    target_nodes: ${graph.data}
+    node_attribute: area_weight
+    scale_dim: 2 # dimension on which scaling applied


This class doesn't have a scale_dim attribute.
It may also be useful to add a general scale by node attribute scaler.

Not yet. Refactor is still ongoing.

training/src/anemoi/training/scaling/variable_level.py

for more information, see https://pre-commit.ci

* reorder-parameter-names-for-plot * use util function to get variable level from metadata if possible

sahahner added 3 commits December 27, 2024 10:15

first version of refactor of variable scaling

511ed18

config training changes

7ddf6d6

avoid multiple scaling

3ddeccc

sahahner linked an issue Dec 27, 2024 that may be closed by this pull request

Loss scalings #5

Open

2 tasks

sahahner linked an issue Dec 30, 2024 that may be closed by this pull request

Pressure Level Scalings only applied in specific circumstances #7

Open

mc4117 reviewed Dec 30, 2024

View reviewed changes

training/src/anemoi/training/train/forecaster.py Outdated Show resolved Hide resolved

mc4117 reviewed Dec 30, 2024

View reviewed changes

training/src/anemoi/training/train/scaling.py Outdated Show resolved Hide resolved

docstring and explain variable reference

be4602c

mc4117 reviewed Dec 31, 2024

View reviewed changes

training/src/anemoi/training/train/forecaster.py Outdated Show resolved Hide resolved

mc4117 added 4 commits December 31, 2024 10:47

fix to config for pressure level scaler

195af07

instantiating scalars as a list

2644c18

preparing for tendency losses

718fc57

Merge branch '7-pressure-level-scalings-only-applied-in-specific-circ…

a34ac02

…umstances' of https://github.com/ecmwf/anemoi-core into 7-pressure-level-scalings-only-applied-in-specific-circumstances

sahahner changed the title ~~pressure level scalings only applied in specific circumstances~~ refactor variable scaling, pressure level scalings only applied in specific circumstances Jan 2, 2025

log the variable level scaling information as before

b91af11

HCookie added the training label Jan 6, 2025

HCookie self-requested a review January 6, 2025 14:36

mc4117 reviewed Jan 7, 2025

View reviewed changes

training/src/anemoi/training/train/scaling.py Outdated Show resolved Hide resolved

pinnstorm added 3 commits January 8, 2025 15:01

adding tendency scaler to additional scalers

c22c50b

reformatting

1f4a532

updating description in configs

2843d98

anaprietonem assigned sahahner Jan 9, 2025

updating var-tendency-scaler spec

c978871

sahahner and others added 4 commits January 22, 2025 15:56

Merge branch '7-pressure-level-scalings-only-applied-in-specific-circ…

61766cd

…umstances' of github.com:ecmwf/anemoi-core into 7-pressure-level-scalings-only-applied-in-specific-circumstances

comment in config file that scler name needs to be added to loss as w…

3adf924

…ell.

fix pre-commit hooks

f19d69d

Merge branch '7-pressure-level-scalings-only-applied-in-specific-circ…

c26d744

…umstances' of https://github.com/ecmwf/anemoi-core into 7-pressure-level-scalings-only-applied-in-specific-circumstances

sahahner mentioned this pull request Jan 23, 2025

Training reorder parameter names for plot #55

Merged

jakob-schloer requested changes Jan 24, 2025

View reviewed changes

training/src/anemoi/training/config/training/default.yaml Outdated Show resolved Hide resolved

jakob-schloer reviewed Jan 24, 2025

View reviewed changes

training/src/anemoi/training/train/forecaster.py Outdated Show resolved Hide resolved

jakob-schloer reviewed Jan 24, 2025

View reviewed changes

training/src/anemoi/training/train/forecaster.py Outdated Show resolved Hide resolved

Update description in training/default

00439cb

HCookie mentioned this pull request Jan 27, 2025

Loss scalings #5

Open

2 tasks

HCookie changed the title ~~fix!: variable scaling, pressure level scalings only applied in specific circumstances~~ fix!: Rework Loss Scalings to provide better modularity Jan 27, 2025

sahahner commented Jan 27, 2025

View reviewed changes

training/src/anemoi/training/train/scaling.py Outdated Show resolved Hide resolved

sahahner and others added 5 commits January 27, 2025 15:45

refactor into training/scaling both the code and the config files, re…

6c857a6

…name all scalars to scalers. move nan-mask for loss function into training/scaling

more scalar renaming to scaler

a2f2728

fix tendency loss

b5f6b5f

fix merge conflict

b5fa55b

[pre-commit.ci] auto fixes from pre-commit.com hooks

cdb9e19

for more information, see https://pre-commit.ci

HCookie reviewed Jan 27, 2025

View reviewed changes

training/src/anemoi/training/scaling/__init__.py Outdated Show resolved Hide resolved

Add '*' to scaler selection.

963c543

HCookie reviewed Jan 27, 2025

View reviewed changes

training/src/anemoi/training/scaling/variable_level.py Outdated Show resolved Hide resolved

HCookie and others added 7 commits January 27, 2025 17:45

Add exclusion of scalers

4f1566b

Fix scalar reference in tests

e4ceb8e

Add all and exclude tests

7178074

[pre-commit.ci] auto fixes from pre-commit.com hooks

08b4cb3

for more information, see https://pre-commit.ci

fix: update all tests, move scaling module into losses

0dbf0b8

print final variable scaling in debug mode

2dccbd2

Training reorder parameter names for plot (#55)

72793f0

* reorder-parameter-names-for-plot * use util function to get variable level from metadata if possible

mc4117 assigned JPXKQX Jan 30, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix!: Rework Loss Scalings to provide better modularity #52

fix!: Rework Loss Scalings to provide better modularity #52

sahahner commented Dec 27, 2024 •

edited by github-actions bot

Loading

b8raoult commented Dec 30, 2024

sahahner commented Dec 30, 2024

FussyDuck commented Jan 2, 2025 •

edited

Loading

JPXKQX commented Jan 8, 2025

mc4117 commented Jan 9, 2025

JPXKQX commented Jan 9, 2025

pinnstorm commented Jan 10, 2025

jakob-schloer commented Jan 24, 2025 •

edited

Loading

jakob-schloer left a comment •

edited

Loading

HCookie Jan 27, 2025

HCookie Jan 27, 2025

sahahner Jan 28, 2025

		@@ -111,25 +134,24 @@ def __init__(

		# Kwargs to pass to the loss function

fix!: Rework Loss Scalings to provide better modularity #52

Are you sure you want to change the base?

fix!: Rework Loss Scalings to provide better modularity #52

Conversation

sahahner commented Dec 27, 2024 • edited by github-actions bot Loading

b8raoult commented Dec 30, 2024

sahahner commented Dec 30, 2024

FussyDuck commented Jan 2, 2025 • edited Loading

JPXKQX commented Jan 8, 2025

mc4117 commented Jan 9, 2025

JPXKQX commented Jan 9, 2025

pinnstorm commented Jan 10, 2025

jakob-schloer commented Jan 24, 2025 • edited Loading

jakob-schloer left a comment • edited Loading

Choose a reason for hiding this comment

HCookie Jan 27, 2025

Choose a reason for hiding this comment

HCookie Jan 27, 2025

Choose a reason for hiding this comment

sahahner Jan 28, 2025

Choose a reason for hiding this comment

sahahner commented Dec 27, 2024 •

edited by github-actions bot

Loading

FussyDuck commented Jan 2, 2025 •

edited

Loading

jakob-schloer commented Jan 24, 2025 •

edited

Loading

jakob-schloer left a comment •

edited

Loading