AHFinderDirect does not mark surfaces as "invalid" after it stops looking for them

Create issue
Issue #2337 wontfix
Roland Haas created an issue

Responding to http://lists.einsteintoolkit.org/pipermail/users/2020-February/007288.html I had a look at AHFInderDirect (see http://lists.einsteintoolkit.org/pipermail/users/2020-February/007291.html) and found this code (in line 678):

674 if ((my_dont_find_after >= 0 and cctk_iteration > my_dont_find_after) or
675     (my_dont_find_after_time > my_find_after_time and cctk_time > my_dont_find_after_time))
676 {
677   assert (! AH_data.search_flag);
678   sf_active[surface_number] = 0;
679 }
680
681 // only try to copy AH info if we've found AHs at this time level
682 if (! AH_data.search_flag) {
683   sf_valid[surface_number] = 0;
684   return;
685 }
686
687 // did we actually *find* this horizon?
688 if (! AH_data.found_flag) {
689   sf_valid[surface_number] = -1;
690   return;
691 }
692
693 sf_active       [surface_number] = 1;

which to me looks like it has sa but in line 678 where there should be a return statement.

Comments (5)

  1. Roland Haas reporter

    A bug, but has been there for a long time. Since it will affect the how surfaces are used by existing runs, will be addressed after the ET_2020_05 release and backported if found to not be too disruptive to code relying on the (erroneous) behaviour.

  2. Roland Haas reporter

    On closer inspecting this may be correct, but benefit from a comment. Namely, not having a return statement in 678 means that (due to the assert in 677) the code is guaranteed to clear sf_valid in 683 and return there.

    Ie the code will mark the surface that was not looked for as invalid.

  3. Log in to comment