search

sandialabs / Albany

Sandia National Laboratories' Albany multiphysics code
Other
272 stars 90 forks source link

landIce_FO_GIS_AdjointSensitivity_Thickness failing again on several platforms #1065

Closed ikalash closed 1 week ago

ikalash commented 1 month ago

Please see: https://sems-cdash-son.sandia.gov/cdash/test/6161385 . It looks like there is a failed comparison.

@mperego @bartgol @mcarlson801

mcarlson801 commented 1 month ago

I'm on it. My fix apparently only fixed it for GPU builds.

ikalash commented 1 month ago

No problem. Thank you @mcarlson801!

mcarlson801 commented 1 month ago

Alright, I think I've got to the bottom of this. I put a bunch of print statements into my kokkos implementation of Response_GLFlux and the original implementation. glflux_debug_output On the left are the values I'm getting with my kokkos implementation and the right are the values from the original implementation. It looks like x and y from the original implementation are Fad types but don't have anything for their derivative values (not even 0s). In the original implementation, x and y are allocated here. My guess is that the dynamic fad size is not getting set since createDynRankView is getting called with a dummy MDField but I'm not 100% sure.

The changes that would have introduced this behavior happened in March of 2021 (https://github.com/sandialabs/Albany/pull/671) and the "correct" sensitivity for this test was added to the test input in October of 2021 (https://github.com/sandialabs/Albany/commit/96889160a25b86549fcea735a46b228289d17136).

@mperego @bartgol @jewatkins Is it possible that the original sensitivity test value is incorrect for this test?

bartgol commented 1 month ago

@mcarlson801 I think your guess on why x/y have no derivs is correct. If you don't pass a valid DataLayout, I think the MDField is not allocated. createDynRankView should only use the input view to deduce the scalar type. But digging a bit in the Sacado code for kokkos, I noticed that ViewFactory::create_view calls dimension_scalar, which ends up calling a specialization of the ViewMapping which does


  // Size of sacado scalar dimension
  KOKKOS_FORCEINLINE_FUNCTION constexpr unsigned dimension_scalar() const
    { return m_fad_size.value+1; }

So if the FAD length was not set, the scalar has length 1.

At this point, I would trust your version.

mcarlson801 commented 1 month ago

@mperego What do you think, should we update the sensitivity test value for this test?

mperego commented 1 month ago

@mcarlson801, I didn't have a chance yet to take a look at that. I'll have a look at it sometime this week. Sorry for the delay

mperego commented 1 week ago

@mcarlson801 Max, sorry it took me so long to get back to this. I think what you have is correct. Thanks for fixing this!

jewatkins commented 1 week ago

@mcarlson801 did you ever make a PR for this fix?

mcarlson801 commented 1 week ago

Posting it now

mcarlson801 commented 1 week ago

Looks like this is clean now, closing.