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

Bugfix on the production of diagnostic variables in rollout_to_netcdf.py + credit.postblock update + other minor bugfix #120

Merged
merged 38 commits into from
Nov 4, 2024

Conversation

yingkaisha
Copy link
Collaborator

@yingkaisha yingkaisha commented Oct 19, 2024

New feature

  • The water budget calculation is split from the GlobalMassFixer; it now has its own GlobalWaterFixer.
  • GlobalMassFixer, GlobalWaterFixer, and GlobalEnergyFixer can now run outside of the model (i.e., they can run inside credit.trainer and rollout_to_netcdf.py).

Bugfix

  • Diagnostic variables can now be produced properly in rollout_to_netcdf.py
  • credit.output now produces files with coordinate names of latitude and longitude rather than 'lat/lon'
  • credit.physcis_constants values were updated to align with CAM6
  • A small bugfix GlobalMassFixer to handle "half level" vertical coordinates

Note

  • The purpose of this PR is to resolve diagnostic-variable-related errors in rollout_to_netcdf.py. It maintains the structure of the current script and touches diagnostic variables only. rollout_to_netcdf.py can be improved fundamentally, but not in this PR.
  • This PR can be verified as follows:
python applications/train.py -c config/example_physics_single.yml
python applications/rollout_to_netcdf.py -c config/example_physics_single.yml

(i.e., produce model weights --> use weights to verify rollout)

@yingkaisha yingkaisha marked this pull request as draft October 19, 2024 01:01
@yingkaisha
Copy link
Collaborator Author

yingkaisha commented Oct 19, 2024

You may see some crossformer.py commits by accident.

I have pushed new commits to align it with the main branch. There are no Wxformer updates in this PR

@yingkaisha yingkaisha changed the title Bugfix on the production of diagnostic variables in rollout_to_netcdf.py + other minor bugfix Bugfix on the production of diagnostic variables in rollout_to_netcdf.py + credit.postblock update + other minor bugfix Oct 21, 2024
@yingkaisha yingkaisha marked this pull request as ready for review October 24, 2024 14:50
@yingkaisha
Copy link
Collaborator Author

Ready

@yingkaisha yingkaisha requested a review from djgagne October 31, 2024 17:47
@yingkaisha
Copy link
Collaborator Author

Merge conflicts resolved

@yingkaisha yingkaisha requested a review from WillyChap November 4, 2024 20:32
Copy link
Collaborator

@djgagne djgagne left a comment

Choose a reason for hiding this comment

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

The new postblock physics and diagnostic updates look good to me. I fixed the CI issue and ran some formatting fixes and removed some unused imports and variables that were flagged by PyCharm. I also want to mention a couple of formatting/style conventions to encourage going forward

  1. We use Google-style docstrings across MILES projects, which follow the following format:
"""
short description of what function/method does

Args
    first_arg (type): blah
    second_arg (type): blah
Returns
    variable (type): description of thing being returned
"""

Please use double-quotes instead of single quotes for consistency purposes. Both types are interchangeable in Python as long as one uses the same type at the beginning and end of a string, but I suspect style guides encourage double quotes because in some languages single quotes can only be used for character types.

@yingkaisha yingkaisha merged commit c6787b6 into main Nov 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants