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

[Roland Haas]

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.

[Roland Haas]

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.

[Roland Haas]

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.

[Roland Haas]

Works as it should.