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

Training reorder parameter names for plot #55

Conversation

sahahner
Copy link
Member

@sahahner sahahner commented Dec 30, 2024

In the newer versions of anemoi-datasets from MARS the variables are not ordered by pressure level anymore but by alphabet, meaning that the order in the loss-plot is also by alphabet (100 is then "smaller" than 20). However, the analysis of the loss weights is more intuitive when ordered by pressure level (or model level).
loss_rstep_rstep00_rank0_epoch085

This PR sorts the variables (for this specific plotting function) in one group by the variable level (if available) interpreting them as digits not strings.

Screenshot 2025-01-29 at 17 40 56

@sahahner sahahner self-assigned this Dec 30, 2024
@@ -824,6 +828,24 @@ def automatically_determine_group(name: str) -> str:
legend_patches,
)

def argsort_name_variablelevel(self) -> list[int]:
Copy link
Member

Choose a reason for hiding this comment

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

Would it be worth putting this in to a callback or loss utils? I can see this function being useful for other callbacks in the future

Copy link
Member Author

Choose a reason for hiding this comment

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

That's right. Will do that.

Copy link
Member Author

@sahahner sahahner Jan 23, 2025

Choose a reason for hiding this comment

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

Should be done after PR #52, where this function is implemented. While for grouping in plots the variable name parsing is still relevant, the variable level should indeed be taken from metadata if possible.

@sahahner sahahner linked an issue Jan 2, 2025 that may be closed by this pull request
@sahahner sahahner changed the base branch from main to 7-pressure-level-scalings-only-applied-in-specific-circumstances January 29, 2025 13:45
@sahahner sahahner requested a review from mc4117 January 29, 2025 13:45
Copy link
Member

@mc4117 mc4117 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@sahahner sahahner merged commit 72793f0 into 7-pressure-level-scalings-only-applied-in-specific-circumstances Jan 30, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Order variables in plotloss callback by pressure level
2 participants