[ET Trac] #2939: AHFinderDirect: range options for track_origin_source_x/y/z

[Jordan Nicoules]

#2939: AHFinderDirect: range options for track_origin_source_x/y/z

 Reporter: Jordan Nicoules
   Status: new
Milestone: 
  Version: 
     Type: bug
 Priority: minor
Component: EinsteinToolkit thorn

In `AHFinderDirect` param.ccl there is (same for y, z):
```
string track_origin_source_x[101] "grid scalar containing the x component of the origin estimate" STEERABLE=recover
{
  "" :: "don't use this feature"
  "[a-zA-Z_][a-zA-Z0-9_]*[:][:][a-zA-Z_][a-zA-Z0-9_]*(\[0-9+\])" :: "name of a grid scalar"
} ""
```

While mimicking this and testing in a thorn of my own, I realized that `""` matches everything, so the second range is basically useless.
**Is it what is meant by** `"don't use this feature"`?
I think I rather understand it as "do not put nothing".

If this is not the intention, first something should be done about the empty string and default value. 
What's more, the range is actually incorrect (see explanation from GPT-5.4 in Copilot below; I'll let you judge the validity of the explanation, you know that better than me).

Possible fixes:

1. For the default: for example use a `"None"` value, with some catch in `AHFinderDirect_setup()` that checks the variable name and returns an error.
With a quick test, an unknown/wrong variable name would trigger an error somewhere in the execution of `AHFinderDirect_find_horizons()` anyway.

2. For the actual range:

* `^.*[:][:].*$` is used in `NSTracker` for example.

* to keep a strict pattern,
```
  "[a-zA-Z_][a-zA-Z0-9_]*[:][:][a-zA-Z_][a-zA-Z0-9_]*"                :: "name of a grid scalar without index"
  "[a-zA-Z_][a-zA-Z0-9_]*[:][:][a-zA-Z_][a-zA-Z0-9_]*\[[0-9][0-9]*\]" :: "name of a grid scalar with index"
```
(GPT's recommendation to separate the two patterns for clarity and portability, instead of trying to match both at the same time).

# Conclusion #

I don't know if there are risks of breaking internal mechanics of AHFinderDirect by changing this.
I would say that the current harm is mostly about false, or misleading, pretenses, as well as a bad template for other people like me.
The possible harm of fixing is to break something else in AHFinderDirect by introducing a different default (this needs verification, but it doesn't look like it from a quick test). There may also be some backwards-compatibility issues if some people managed to use grid scalars that don't match the new pattern.

## GPT's explanation

>Yes. The old regex from param.ccl is not actually matching PunctureTracker::pt_loc_y[0] as written.
>
>The key point is that Cactus string ranges use POSIX regexes, and the docs explicitly say extended regex support is not portable. In practice, you should write these as portable POSIX-BRE-style patterns, not PCRE-ish ones.
>
>Your old pattern is:
>
> `[a-zA-Z_][a-zA-Z0-9_]*[:][:][a-zA-Z_][a-zA-Z0-9_]*(\[0-9+\])`
>
>Why it fails:
>
>`\[` and `\]` correctly mean literal [ and ].
>But 0-9+ is outside a character class, so it means the literal characters 0, -, 9, +, not “one or more digits”.
>The surrounding ( ) are also suspicious, because in POSIX basic regex they are literals unless escaped as`\(` and `\)`. So they are not doing grouping in the portable interpretation.
So that pattern is effectively trying to match something like:
>
> `thorn::var([0-9+])`
>
>or at best a very literal bracket payload, not `thorn::var[0]`.
>
>That is why `""` made it “work”: per the Cactus docs, the empty regex matches anything. So before, validation was effectively bypassed.
>
>What you want, portably, is something like:
>
> `[a-zA-Z_][a-zA-Z0-9_]*[:][:][a-zA-Z_][a-zA-Z0-9_]*\[[0-9][0-9]*\]`
>
>That matches:
>
> * implementation name
> * ::
> * variable name
> * [ digits ]
>
>including `PunctureTracker::pt_loc_y[0]`.

--
Ticket URL: https://bitbucket.org/einsteintoolkit/tickets/issues/2939/ahfinderdirect-range-options-for
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.einsteintoolkit.org/pipermail/trac/attachments/20260508/5f7c8f5b/attachment.htm>