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

Issue649 addd diagnostic fsd #661

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

docguibou
Copy link
Contributor

Addition of diagnostic FSD computed online. It compiles and runs. It involved a bit of cleaning of the "old" FSD computation and adding floe size parameters as outputs.

Copy link
Contributor

@tdcwilliams tdcwilliams 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 @docguibou - some minor changes needed perhaps

double M_0 = 0, M_1 = 0., M_neg1 = 0., M_neg2 = 0.;
if (M_conc_mech_fsd[M_num_fsd_bins-1][i]<=M_dmax_c_threshold*D_conc[i])
{
//If more than 90% of sea ice that is in the element is "broken"
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add an explanation for the ordering? It looks like you start at the largest floe size bin and decrease - what is the reason for that? (I would expect the other way so if 90% of the ice is in the smallest bin you'd stop there with a small Dmax, otherwise keep increasing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So happy you're back! Hmmm I haven't changed the ordering actually. I suppose my reasoning at the time was that there is more elements with unbroken ice than with broken ice, so the loop would stop faster starting from the larger floes. If it really does not make sense to you, I can rewrite it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I am misunderstanding, the current Dmax comes from \int_{Dmax}^\infty = 0.9, but reversing gives \int_0^{Dmax} = 0.9? These are quite different. But perhaps we could think about it and if it is important and change it in the other branch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I defined M_dmax_c_threshold as (1-0.9) to account for that, that's ok no? Super confusing with these comments though.

model/finiteelement.cpp Show resolved Hide resolved
model/finiteelement.cpp Outdated Show resolved Hide resolved
model/finiteelement.hpp Outdated Show resolved Hide resolved
model/gridoutput.hpp Outdated Show resolved Hide resolved
model/model_variable.cpp Show resolved Hide resolved
Copy link
Contributor

@tdcwilliams tdcwilliams left a comment

Choose a reason for hiding this comment

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

I made some suggestions for doxygen comments which should be committed, and another couple, which you can decide about

model/finiteelement.cpp Outdated Show resolved Hide resolved
model/finiteelement.cpp Outdated Show resolved Hide resolved
model/finiteelement.cpp Show resolved Hide resolved
model/finiteelement.cpp Show resolved Hide resolved
M_0 = D_conc[i];
// No need to normalize the other moments as we do a ratio
// Now compute the characteristic floe sizes
D_dmean[i] = M_1_mech / M_0; // Characteristic floe size associated with the number of floes
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like D_dchar but for mech FSD?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is! I am reluctant to change that as it is what we published, but to be consistent with assumptions from scattering parameterizations, I should have not used the areal FSD. However, that would be assuming that absolutely all floes contribute to scattering, which is likely to be wrong. I can discuss that with you at some point.

double M_0 = 0, M_1 = 0., M_neg1 = 0., M_neg2 = 0.;
if (M_conc_mech_fsd[M_num_fsd_bins-1][i]<=M_dmax_c_threshold*D_conc[i])
{
//If more than 90% of sea ice that is in the element is "broken"
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I am misunderstanding, the current Dmax comes from \int_{Dmax}^\infty = 0.9, but reversing gives \int_0^{Dmax} = 0.9? These are quite different. But perhaps we could think about it and if it is important and change it in the other branch

docguibou and others added 3 commits January 6, 2025 13:34
Co-authored-by: Timothy Williams <[email protected]>
Co-authored-by: Timothy Williams <[email protected]>
Co-authored-by: Timothy Williams <[email protected]>
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