sandialabs / Albany

Sandia National Laboratories' Albany multiphysics code
Other
282 stars 89 forks source link

Failing enthalpy performance test on weaver #966

Open jewatkins opened 1 year ago

jewatkins commented 1 year ago

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.

jewatkins commented 1 year ago

Passed on May 10: https://sems-cdash-son.sandia.gov/cdash/viewTest.php?onlypassed&buildid=48324 Failed on June 6: https://sems-cdash-son.sandia.gov/cdash/viewTest.php?onlyfailed&buildid=49253 Might be related to this change: https://github.com/sandialabs/ali-perf-tests/commit/e55481065a6ed62b8a2344ca005e5ac0d5e646ae#diff-8c2b98749c573451feece7bacecce357e7e48dc059cff41f7a814d069aae429fR23

mperego commented 1 year ago

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 commented 1 year ago

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 commented 12 months ago

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 commented 12 months ago

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 commented 12 months ago

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. https://github.com/sandialabs/Albany/commit/c139d04a9dd7a2249d68da749f05609a12fc6205#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 commented 12 months ago

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

jewatkins commented 12 months ago

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