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

Failing enthalpy performance test on weaver #966

Open
jewatkins opened this issue Jun 7, 2023 · 8 comments
Open

Failing enthalpy performance test on weaver #966

jewatkins opened this issue Jun 7, 2023 · 8 comments

Comments

@jewatkins
Copy link
Collaborator

Seem like the value has changed: https://sems-cdash-son.sandia.gov/cdash/test/3496757
Once blake performance tests are back up, maybe we can revisit. Might just need to rebless.

@mperego
Copy link
Collaborator

mperego commented Nov 22, 2023

Oh, yeah.. not sure why I changed that value. can you try setting the Clausius Clapeyron coefficient back to
Clausius-Clapeyron Coefficient: 7.9e-08 ?
Is this test only run on Weaver? I thought it had to do with GPUs

@jewatkins
Copy link
Collaborator Author

jewatkins commented Nov 22, 2023

yeah it's the only failing test on weaver at the moment. I'll try changing it back. It actually looks like blake failed in the same way too on June 22 when it was temporarily back up: https://sems-cdash-son.sandia.gov/cdash/test/3533581

jewatkins added a commit to sandialabs/ali-perf-tests that referenced this issue Nov 22, 2023
@jewatkins
Copy link
Collaborator Author

Reverting the coefficient didn't seem to fix the issue but the value is closer:

Old blake value: -6.999302004118e+00
Old weaver value: -6.999302007624e+00
New blake value: -7.183956409193e+00
New weaver value: -7.183955401688e+00
Test value: -7.152645593729e+00

I don't see anything else that was changed in the input file so maybe it was an Albany change? @mperego should we just rebless? There are a lot of other blake tests that will be failing comparisons soon too:

2:green-1-10km_ent_muk_np384
5:green-1-10km_ent_mu_np384
9:green-3-20km_vel_muk_np384
12:green-3-20km_ent_muk_np384
13:green-3-20km_vel_mu_np384
14:green-3-20km_beta_1ws_np384
15:green-3-20km_beta_mem_np384
16:green-3-20km_beta_memp_np384
20:humboldt-1-10km_ent_mu_np384
21:humboldt-1-10km_cop_if2_np384
22:humboldt-1-10km_cop_fro_np384
25:thwaites-1-10km_ent_mu_np384

The beta analysis ones are segfaulting so not sure if the value has changed.

@mperego
Copy link
Collaborator

mperego commented Nov 28, 2023

I remember now why I set the Clausius-Clapeyron Coefficient to zero. There was a bug in the Enthalpy solver where the Clausius Clapayron cofficient was not used to compute the viscosity (#955).
There must be something else that happened on the Albany side. It would be good to understand what caused that difference but it might not be worth it.

@jewatkins
Copy link
Collaborator Author

Unfortunately, we have new hardware on blake and weaver had an os upgrade in that timeframe so I'm not sure we can recover the original values.

There were a lot of changes (including Trilinos). The only one that might be relevant is this one.
c139d04#diff-45c3d05f395e38ab4eaba33a43be27a3185d24a96b9f1664b35441f5c4775580R22
I don't see that parameter in the enthalpy tests. But it looks like the default is true and it should recover old behavior?

@mperego
Copy link
Collaborator

mperego commented Nov 29, 2023

Yes, I think that the default should recover the old behavior.
It's fine with me if you simply re-bless it.

@jewatkins
Copy link
Collaborator Author

Okay. I'll rebless as I create the new tests.

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

No branches or pull requests

2 participants