sandialabs / LCM

Laboratory for Computational Mechanics
Other
12 stars 7 forks source link

FPEs in some of the ACE nightlies #57

Closed ikalash closed 2 years ago

ikalash commented 2 years ago

Looks like there are FPEs in some of the ACE nightlies starting today: https://sems-cdash-son.sandia.gov/cdash/index.php?project=Albany_LCM&date=2022-03-25&filtercount=1&showfilters=1&field1=groupname&compare1=61&value1=Nightly . @lxmota , could this be due to your cosmetic changes check-in yesterday? Hopefully it's that and not something in Trilinos.

lxmota commented 2 years ago

@ikalash @jennifer-frederick

I reverted my cosmetic changes of yesterday to make sure they weren't the cause of the FPEs. The problem is in the following line:

https://github.com/sandialabs/LCM/blob/main/src/LCM/evaluators/ACEThermalParameters_Def.hpp#L299

The variable qebt is about 3.9e+32 and v is about 0.1, so qebt ends up being raised to the 10th power, which is out of the representation range for doubles, causing the FPE.

But I don't know why this started happening. After reverting my cosmetic changes, the last changes are from February.

ikalash commented 2 years ago

Thanks @lxmota . Lets wait till the next time the tests run to see what happens, and go from there.

ikalash commented 2 years ago

Ok, it wasn't your cosmetic changes, as we are still seeing the FPEs. I wonder if something changed in Trilinos to cause the problem, but in any case, if we know the cause is qebt = O(1e32), which would result in overflow, maybe that needs to be set to a different value? Bringing @jennifer-frederick into the discussion.

ikalash commented 2 years ago

Thanks @jennifer-frederick ! I'll go ahead and close this tomorrow if the nightly tests reveal that the issue has been fixed.

ikalash commented 2 years ago

It looks like the FPEs gone, but some tests are diff'ing. @jennifer-frederick I assume you changed some of the parameter values and this is likely why they are diff'ing? If so, I will just rebaseline the tests affected.

jennifer-frederick commented 2 years ago

Yes, the fix to the overflow issue did change the value for ice saturation very slightly. I would expect some differences, but nothing large. If the differences are small, then rebaselining the tests is ok!

From: Irina K. Tezaur @.> Sent: Tuesday, March 29, 2022 9:43 AM To: sandialabs/LCM @.> Cc: Frederick, Jennifer M @.>; Mention @.> Subject: [EXTERNAL] Re: [sandialabs/LCM] FPEs in some of the ACE nightlies (Issue #57)

It looks like the FPEs gone, but some tests are diff'ing. @jennifer-frederickhttps://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fjennifer-frederick&data=04%7C01%7Cjmfrede%40sandia.gov%7Ce4baa0e2d67945f400fd08da11a32f46%7C7ccb5a20a303498cb0c129007381b574%7C1%7C0%7C637841689821306267%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=yo%2BwX%2BRsZ9KZC8H1ZRdMEn%2FklKLTgIQ91wL11lIeiwA%3D&reserved=0 I assume you changed some of the parameter values and this is likely why they are diff'ing? If so, I will just rebaseline the tests affected.

- Reply to this email directly, view it on GitHubhttps://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fsandialabs%2FLCM%2Fissues%2F57%23issuecomment-1082111528&data=04%7C01%7Cjmfrede%40sandia.gov%7Ce4baa0e2d67945f400fd08da11a32f46%7C7ccb5a20a303498cb0c129007381b574%7C1%7C0%7C637841689821306267%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=nDEYUJQiED2QNoLc23iGE3baPXxDpu18BB7%2F2n2ufDA%3D&reserved=0, or unsubscribehttps://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAH4YZ73WMTCTHAQONTBVTPDVCMXHNANCNFSM5RU24IFA&data=04%7C01%7Cjmfrede%40sandia.gov%7Ce4baa0e2d67945f400fd08da11a32f46%7C7ccb5a20a303498cb0c129007381b574%7C1%7C0%7C637841689821306267%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=z7npdd3Ajz%2FwQSW2OGnEk492%2FIZquaHWL5n3hv5P%2BTQ%3D&reserved=0. You are receiving this because you were mentioned.Message ID: @.**@.>>

ikalash commented 2 years ago

The changes are minor. I will go ahead and rebaseline then, thanks!