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

[WIP] Rayleigh Damping mixed precision fix #32

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

FlorianDeconinck
Copy link
Collaborator

  • Move RFF calculation to f64 to match Fortran
  • Fix the implementation check for RayleighDamping and move it closer to the code

⚠️ This cannot be merged as of 2024/11/29 ⚠️

  • The mixed precision system relies on features that are not yet merged in mainline gt4py but only available on the experimental NASA branch
  • To be tested, this requires a f32 dataset

Fix the implementation check for RayleighDamping and move it closer to the code
@FlorianDeconinck
Copy link
Collaborator Author

Poking @oelbert to show mixed precision in dycore in action

@@ -18,7 +19,7 @@
from ndsl.dsl.typing import Float, FloatField, FloatFieldK


SDAY = 86400.0
SDAY = Float(86400.0)
Copy link

Choose a reason for hiding this comment

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

If seconds per day is used frequently in the the ndsl, dycore, and physics, would it be better to define it in the ndsl constants section to ensure uniformity?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes we probably should be moving that to constant

@@ -36,7 +37,7 @@ def compute_rf_vals(pfull, bdt, rf_cutoff, tau0, ptop):
@gtscript.function
def compute_rff_vals(pfull, dt, rf_cutoff, tau0, ptop):
rffvals = compute_rf_vals(pfull, dt, rf_cutoff, tau0, ptop)
rffvals = 1.0 / (1.0 + rffvals)
rffvals = f64(1.0) / (f64(1.0) + rffvals)
Copy link

Choose a reason for hiding this comment

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

In an f32 run, aren't you only promoting the constants here and not rffvals?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The upcaster will force in the rffvals to be f64. Alternatively you could, type hint rffvals e.g.

rffvals: f64 = ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The upcaster follows basic C up-casting rules

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