-
Notifications
You must be signed in to change notification settings - Fork 75
Revert agnosticity and make Surface Water Mass Transformation Recipe MOM6 specific #623
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
base: main
Are you sure you want to change the base?
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
is this ready to be reviewed @willaguiar? |
|
Yes! |
| @@ -8,7 +8,7 @@ | |||
| "\n", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOVE THE TABLE
Given that we remove the client info (I like), should we also include here session requirements? I.e. choose a large one?
Reply via ReviewNB
| @@ -8,7 +8,7 @@ | |||
| "\n", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line #20. warnings.filterwarnings("ignore", category = FutureWarning)
Apparently this is bad practice and we should not use :(
Reply via ReviewNB
| @@ -8,7 +8,7 @@ | |||
| "\n", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the functions are not the clearer way to do this? but I think it might be too much for this pull request, it can be done after this is merged!
Reply via ReviewNB
| @@ -8,7 +8,7 @@ | |||
| "\n", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these values represent upwelling/downwelling? My understanding was that positive means waters becoming denser, negative waters become lighter. If they are dense enough or not to subduct depends on surrounding stratification. And then because this is at the surface becoming lighter does not trigger upwelling. But maybe I'm wrong?
Reply via ReviewNB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having done a bit of reading the magnitude that is related to upwelling/downwelling is the derivative of the transformation rate with respect to density (
I'm not sure if this is correct then, but would the amount upwelled would then be 30Sv? Need @adele-morrison @AndyHoggANU to help me jaja
| @@ -8,7 +8,7 @@ | |||
| "\n", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe comment in line about this being an integration? Multiplying by area and summing?
Reply via ReviewNB
| @@ -8,7 +8,7 @@ | |||
| "\n", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @@ -8,7 +8,7 @@ | |||
| "\n", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes issue #590
Notebook converted back from model-agnostic to MOM6-based. Specific additional directions are given on how to alter the notebook to work with MOM5 (ACCESS-OM2-01). An additional if statement was added, to convert the temperature for potential to conservative, only when the input variable is potential temperature. This line comes with a warning, and was added so the code can work correctly by just changing the diagnostic variables for MOM5 ( i.e., the conditional only perform the conversion when the output is named according to the MOM6 standard naming for potential density)