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

Pressure Level Scalings only applied in specific circumstances #7

Open
mc4117 opened this issue Dec 10, 2024 · 5 comments
Open

Pressure Level Scalings only applied in specific circumstances #7

mc4117 opened this issue Dec 10, 2024 · 5 comments
Assignees
Labels
bug Something isn't working training

Comments

@mc4117
Copy link
Member

mc4117 commented Dec 10, 2024

What happened?

Pressure level scalings are only applied if the variable is specified in the variable loss scaling. This is because of this line https://github.com/ecmwf/anemoi-training/blob/develop/src/anemoi/training/train/forecaster.py#L303

What are the steps to reproduce the bug?

Specify for example

training:
pl:
t: 1

in the training config. You will then see that only the temperature is scaled by pressure level.

Version

0.3.1

Platform (OS and architecture)

Relevant log output

No response

Accompanying data

No response

Organisation

No response

@mc4117 mc4117 added the bug Something isn't working label Dec 10, 2024
@HCookie
Copy link
Member

HCookie commented Dec 10, 2024

What should be the behaviour if not specified? It seems that not doing anything is the appropriate response

@mc4117
Copy link
Member Author

mc4117 commented Dec 10, 2024

I think whether you scale the loss across pressure levels should be independent of whether you use the default loss scaling for the variable.
My proposal would be something like

pressure_level_scaler:
  _target_: anemoi.training.data.scaling.ReluPressureLevelScaler
  minimum: 0.2
  slope: 0.001
  variables: [t, z, u, v, w, q]

@HCookie
Copy link
Member

HCookie commented Dec 10, 2024

So, you want the pressure level scaling independant of loss_scaling?
I believe we already have the instantiate like you suggest.
So a user in the config should be able to:

loss:
    _target_: anemoi.training.losses.mse.WeightedMSE
   scalars: ['variable', 'pressure_level']
  # or 
   scalars: ['pressure_level']

@mc4117
Copy link
Member Author

mc4117 commented Dec 10, 2024

That's a cool functionality! But I'm not sure it currently works given https://github.com/ecmwf/anemoi-training/blob/develop/src/anemoi/training/train/forecaster.py#L303?

@HCookie
Copy link
Member

HCookie commented Dec 10, 2024

But I'm not sure it currently works

It was a suggestion, yes, it won't currently work, and will require a refactor.

@mc4117 mc4117 mentioned this issue Dec 19, 2024
2 tasks
@JesperDramsch JesperDramsch transferred this issue from ecmwf/anemoi-training Dec 19, 2024
@sahahner sahahner self-assigned this Dec 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working training
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

4 participants