landIce_FO_GIS_AdjointSensitivity_Thickness failing again on several platforms

[ikalash]

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

@mperego @bartgol @mcarlson801

[mcarlson801]

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

[ikalash]

No problem. Thank you @mcarlson801!

[mcarlson801]

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.

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 (#671) and the "correct" sensitivity for this test was added to the test input in October of 2021 (9688916).

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

[bartgol]

@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]

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

[mperego]

@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]

@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]

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

[mcarlson801]

Posting it now

[mcarlson801]

Looks like this is clean now, closing.