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

MOM_interface PR #190 Broke Dimensional Consistency test #302

Closed
alperaltuntas opened this issue Sep 6, 2024 · 2 comments · Fixed by #318
Closed

MOM_interface PR #190 Broke Dimensional Consistency test #302

alperaltuntas opened this issue Sep 6, 2024 · 2 comments · Fixed by #318

Comments

@alperaltuntas
Copy link
Member

alperaltuntas commented Sep 6, 2024

PR #190 Broke Dimensional Consistency test for the T (time) dimension. All other dimensions are still passing.

@alperaltuntas
Copy link
Member Author

alperaltuntas commented Oct 2, 2024

A bit more info: The culprit appears to be FULL_DEPTH_KHTR_MIN. Setting it to False fixes the failing dimensional consistency test T. Looking at the checksums of a scaled run with FULL_DEPTH_KHTR_MIN=True, the first diff appears in MOM_diabatic driver, after KPP, when compared to a non-scaled run with FULL_DEPTH_KHTR_MIN=True.

@iangrooms
Copy link

On these lines the CS%Khtr_min is compared against Coef_x and Coef_y, but I'm pretty sure that the dimensions of Coef_x and Coef_y do not match the dimensions of CS%Khtr_min.

Looking just a few lines up

Coef_y(i,J,K) = I_numitts * khdt_y(i,J)
Coef_x(I,j,K) = I_numitts * khdt_x(I,j)

I_numitts is nondimensional while khdt_{x,y} are, as the name suggests, a diffusivity times the time step. In light of this, I think the following might fix the dimensional consistency problem:

Coef_y(i,J,K) = max(Coef_y(i,J,K), dt * CS%KhTr_min)
Coef_x(I,j,K) = max(Coef_x(I,j,K), dt * CS%KhTr_min)

This would change the results substantially. I haven't tested it.

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 a pull request may close this issue.

2 participants