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

Vertically-integrated frazil_3d diagnostic #291

Merged
merged 3 commits into from
Jun 28, 2019

Conversation

rmholmes
Copy link
Contributor

This commit adds a diagnostic frazil_3d_int_z that vertically sums the frazil_3d diagnostic for space saving (note this is not the same as frazil_2d, which takes only the surface value). See discussion here.

Successfully tested in a 1-degree ACCESS-OM2 run.

@aidanheerdegen
Copy link
Contributor

I'm fixing the travis CI compilation tests in this PR #292

In the meantime, @rmholmes have you tested this and confirmed that your diagnostic is the same as the 3d one when that is summed in post processing?

@aidanheerdegen aidanheerdegen self-assigned this Jun 24, 2019
Copy link
Collaborator

@russfiedler russfiedler left a comment

Choose a reason for hiding this comment

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

Need to change the description of the diagnostic.
This isn't the vertical integral of the heat flux. It's the sum of the heat fluxes or the integral of the heat changes due to the formation of frazil.

Otherwise looks good.

@rmholmes
Copy link
Contributor Author

@russfiedler Right thanks. I've changed "z-integral" to "Vertical sum".

@aidanheerdegen yes I ran one year of 1deg_jra55_ryf, vertically-summed frazil_3d using ncap2 and it matched the frazil_3d_int_z diagnostic to within +-2e-6 Wm-2 (where the diag itself is ~1-30Wm-2).

One thing that I wasn't 100% sure of -> The part inside the if (pop_icediag). Does this go to the ice model diagnostics?

@russfiedler
Copy link
Collaborator

@rmholmes Yes it does, so you have to do the calculation in two places like you've done.

@aekiss
Copy link
Collaborator

aekiss commented Jun 25, 2019

Thanks @rmholmes.

For clarity, should the description for frazil_2d be changed from

'ocn frazil heat flux over time step'

to something like

'ocn frazil heat flux from the top level over time step'

?

@rmholmes
Copy link
Contributor Author

Good point @aekiss, I've changed it.

@aekiss
Copy link
Collaborator

aekiss commented Jun 27, 2019

@russfiedler, @aidanheerdegen - if this PR looks OK, can one of you pls merge it in so I can compile new exes for Andy before the weekend? Ta!

@aidanheerdegen
Copy link
Contributor

Will be in tomorrow, will check it then and merge if all ok. I borked the CI checks and can't get them back online.

@aidanheerdegen
Copy link
Contributor

@rmholmes can you rebase this on to the current master branch so the travis tests will fire?

@aidanheerdegen
Copy link
Contributor

Should just be a change to the .travis.yml file in the top level

@aidanheerdegen
Copy link
Contributor

@rmholmes Don't worry. Seemed to have trigged them with the test PR I did. All good.

@aidanheerdegen aidanheerdegen merged commit 50dc61e into mom-ocean:master Jun 28, 2019
@rmholmes rmholmes deleted the frazil3d_int_z_diag branch July 1, 2019 03:45
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.

4 participants