mhd_eigen values are unused

[zingale]

In computing the eigensystem of the MHD equations in mhd_eigen.H, we have logic like:

  if (std::abs(cfz-csz) <= 1e-14){
    alf = 1.0_rt;
    als = 0.0_rt;
  }

  if (as - csz < 0.0) {
    alf = 0.0_rt;
  } else {
    alf = std::sqrt((as - csz)/(cfz - csz));
  }

  if (cfz - as < 0.0) {
    als = 0.0_rt;
  } else {
    als = std::sqrt((cfz - as)/(cfz - csz));
  }

So the values set in the first test are never used, since they are overwritten by the else in the next 2 tests.

This was identified by clang-tidy

[guadabsb15]

Ah, I see! what clang-tidy says makes a lot of sense!

So I think the origin of that first if (std::abs(cfz-csz) <= 1e-14) comes from Stone et.al 2018 paper (https://iopscience.iop.org/article/10.1086/588755). On page 171, it says "In the degenerate case in which Ca=Cax=a, so cfz =csz , then equation (A16) becomes alf= 1 and als= 0."

So the question is, if that first if is removed, then for that case when cfz=csz , would these terms alf and als approach the "theoretical" 1 and 0 or numerically have values far from it? or should the second and third ifs only be done if cfz is "significantly" different than csz and then avoid overwriting?

I'm not sure if trying to not overwrite the first if matters, since the tests in https://github.com/AMReX-Astro/Castro/pull/2880 are passing

[guadabsb15]

I just saw the comment on the corresponding file of https://github.com/AMReX-Astro/Castro/pull/2880 addressing this and the suggested change solves this already

[zingale]

yeah, I think the PR fixes this issue.