MoL RK87 non-functional

Issue #1504 closed
Roland Haas created an issue

currently (Noether release and trunk, since rev 190) MoL's RK87.c file contains a line (line 234):

  CCTK_WARN(0, "Peter has been too lazy to write the RK87 routine "
            "out for array variables. Better send him an email...");

which unconditionally aborts the run whenever RK87 is used, even when used for grid functions and not grid arrays. The fix is obviously to check MoLNumEvolvedArrayVariables before aborting:

  if (MoLNumEvolvedArrayVariables > 0 ||
      MoLNumEvolvedComplexArrayVariables > 0)
  {
    CCTK_WARN(0, "Peter has been too lazy to write the RK87 routine "
              "out for array variables. Better send him an email...");
  }

Keyword: MoL
Keyword: backport

Comments (13)

  1. Frank Löffler
    • removed comment

    It is. However, it is sad that this was not triggered by a testsuite, which quite clearly means that there is no testsuite that uses RK78. Please backport that proposed change to the release (after testing it, obviously).

    After that, we should think about an easy way to test all methods currently implemented in MoL - one short and small, but yet meaningful testsuite for each of them. What about using the testsuite of CactusExamples/WaveMoL as template?

  2. Roland Haas reporter
    • changed status to open
    • removed comment

    Yes, I think the lesson to learn from this is to make sure we have better test coverage. The RK87 method is apparently used, I suspect because it is a variable timestep method and variable timesteps are only useful with PUGH. This actually worked before and we missed the regression in the review :-).

    Did we already agree on a way to tag fixes for backporting? Using "Please backport" in the ticket? Adding a "Backport" keyword?

  3. Frank Löffler

    I think variable time steps are even usable in Carpet, at least to some extend. We don't have a defined way to tag backports - as it didn't usually need to happen so often. I suggest to use the relevant milestone tag, which I now do here.

  4. Peter Diener
    • removed comment

    RK87 can also be used with fixed timesteps. But yes, variable timesteps can be used in Carpet, for example when using multiblock and no mesh refinement. This was where it was used initially.

  5. Roland Haas reporter
    • removed comment

    Replying to [comment:5 knarf]:

    I think variable time steps are even usable in Carpet, at least to some extend. We don't have a defined way to tag backports - as it didn't usually need to happen so often. I suggest to use the relevant milestone tag, which I now do here.

    How did you manage to set a past milestone? I tried doing that but could not find ET_2013_11 in the drop down list of milestones in the Modify Ticket section.

  6. Roland Haas reporter
    • changed status to resolved
    • removed comment

    fixed in rev 205 of MoL backported to Noether in rev 206.

    Re comment:8 : using Version we can only tag a single version so we'd first fix it in trunk (leaving the version at the default) the change the version to the release. This does not seem ideal. It would seem better if one could flag an important ticket for backporting right away.

  7. Log in to comment