problem with runtool update

[kglotfelty]

I hit an error running regression tests this week w/ the changes in #815 . The csmooth sclmax parameter with the parameter redirect isn't working.

from ciao_contrib.runtool import make_tool

csmooth = make_tool("csmooth")

csmooth.infile = "/export/img.fits"
csmooth.outfile = "/pool1/kjg/out.img"
csmooth.sclmin = 1
csmooth.sclmax = 10
csmooth(clobber=True)

gives

%  python boo.py 
Traceback (most recent call last):
  File "/home/kjg/boo.py", line 8, in <module>
    csmooth.sclmax = 10
    ^^^^^^^^^^^^^^
  File "/export/ciaox/lib/python3.11/site-packages/ciao_contrib/runtool.py", line 1107, in __setattr__
    self._validate(pname, val, store=True)
  File "/export/ciaox/lib/python3.11/site-packages/ciao_contrib/runtool.py", line 1050, in _validate
    if not self._check_param_limit(pname, pval, pinfo.lo, operator.gt):
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/export/ciaox/lib/python3.11/site-packages/ciao_contrib/runtool.py", line 1683, in _check_param_limit
    return pval == lim or op(pval, lim)
                          ^^^^^^^^^^^^^
TypeError: '>' not supported between instances of 'float' and 'str'

if I take out the sclmax line it runs.

[DougBurke]

Darn it. I was thinking it was becasue of #815 (comment) but after 2s of reflection I think it's because sclmni and sclmax are both set to INDEF for csmooth and I obviously haven't trapped that case well in the new code. It looks like I just need to change INDEF to None in this case,as the CIAO 4.15 code has

>>> from ciao_contrib import runtool as rt
>>> print(rt.csmooth.sclmin)
None
>>> print(rt.csmooth.sclmax)
None

[DougBurke]

Actually, it is related to my earlier concern. Here's the CIAO 4.15 code:

sherpa-4.15.0> rt.csmooth._store["sclmin"]
ParValue(name='sclmin', type='r', help='initial (minimal) smoothing scale [pixel]', default=None)

sherpa-4.15.0> rt.csmooth._store["sclmax"]
ParRange(name='sclmax', type='r', help='maximal smoothing scale [pixel]', default=None, lo=')sclmin', hi=None)

whereas we now have

sherpa-4.15.1+367.g9fe5a3b8> rt.csmooth._store['sclmin']
ParValue(name='sclmin', type='r', help='initial (minimal) smoothing scale [pixel]', default=None)

sherpa-4.15.1+367.g9fe5a3b8> rt.csmooth._store['sclmax']
ParRange(name='sclmax', type='r', help='maximal smoothing scale [pixel]', default=None, lo='INDEF', hi=None)

The problem is that the sclmax.p_max value has lost the redirect here.

[DougBurke]

So, we can

• revert the change to using paramio
• just drop the redirect handling for the min/max values, as there are only a few par files that take advantage of it

I'm leaning towards the former but open to input.

[kglotfelty]

I'm not sure what it does with the min/max redirects ... if all it does is additional work to evaluate them then I'm not sure it's worth it. It look like it's checked when the parameter is set rather than when the tool is run ... so order matters ...

so if i set the max before the min, I can set the max to anything and an error is raised when the tools is run:

>>> from ciao_contrib.runtool import csmooth
>>> csmooth.sclmax = 1
>>> csmooth.sclmin = 10
>>> csmooth()
pset: parameter below minimum : sclmax
/tmp/tmp3cuuw3xx.csmooth.par: parameter below minimum : sclmax
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/export/ciao_from_source/ciao-4.15/contrib/lib/python3.9/site-packages/ciao_contrib/runtool.py", line 1836, in __call__
    stackfiles = self._update_parfile(parfile)
  File "/export/ciao_from_source/ciao-4.15/contrib/lib/python3.9/site-packages/ciao_contrib/runtool.py", line 1396, in _update_parfile
    self._update_parfile_verify(parfile, stackfiles)
  File "/export/ciao_from_source/ciao-4.15/contrib/lib/python3.9/site-packages/ciao_contrib/runtool.py", line 1326, in _update_parfile_verify
    oval = _to_python(ptype, pio.pget(fp, oname))
ValueError: pget() Parameter not found

vs. if I set the min first, then the value is validated

>>> from ciao_contrib.runtool import csmooth
>>> csmooth.sclmin=10
>>> csmooth.sclmax = 1
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/export/ciao_from_source/ciao-4.15/contrib/lib/python3.9/site-packages/ciao_contrib/runtool.py", line 1107, in __setattr__
    self._validate(pname, val, store=True)
  File "/export/ciao_from_source/ciao-4.15/contrib/lib/python3.9/site-packages/ciao_contrib/runtool.py", line 1051, in _validate
    raise ValueError(f"{self._toolname}.{pname} must be >= {pinfo.lo} but set to {pstr}")
ValueError: csmooth.sclmax must be >= )sclmin but set to 1

so what it's currently doing looks a little fragile.

A quick grep through and it looks like csmooth is the only tool with redirects in the min/max. [And I'm not really sure why, but tgdetect has literal INDEFs in the min & max for the xoffset and yoffset parameters ... which doesn't really make sense.]

[DougBurke]

The current design is to apply the constraint (on min/max) when the parameter is set, not when the command is actually run, I think but haven't checked. I could also add it at run time. Not that that helps with this particular issue.

[kglotfelty]

If we don't think there's a fix coming can we revert this back to the original? This casues the dax regression tests to die badly.