wfc3tools incorrect parameter symmetry

[mdlpstsci]

Some of the wf3tools (Python) present a different interface than their C counterparts. The Python wrappers which exhibit this behavior should be updated to reflect the underlying C interface.
For example, wf3rej.py has explicit checking for input lists and raises an exception if a list is passed in as this shows here.

if len(parseinput.irafglob(input)) > 1:
        raise IOError("wf3rej can only accept 1 file for"
"input at a time: {0}".format(infiles))

The documentation implies input lists are allowed.

input : str, Name of input files

      - a single filename (``iaa012wdq_raw.fits``)
      - a Python list of filenames                            <== HERE
      - a partial filename with wildcards (``\*raw.fits``)
      - filename of an ASN table (``\*asn.fits``)
      - an at-file (``@input``)

This can be fixed by removing the check and exception. The underlying C executable, wf3rej.e, can handle input lists, but it requires the input to be a CSV string. This needs to be fixed in the Python wrapper (e.g., here in wf3rej.py)

call_list.append(input)

should be

input_string_list = ','.join(input)
call_list.append(input_string_list)

This was introduced in #13.

Code Changes

List is also evident in other tools and should be addressed.

• wf3rej.py TESTING https://github.com/spacetelescope/wfc3tools/compare/master...mdlpstsci:issue22_inputlist?expand=1
• calwf3.py
• TBD

Doc Changes

• calwf3
• wf3ref
• TBD

[sosey]

You should verify, but my memory is that the C executables only accept a single file as input (either a observation or an association table). The pyraf wrapper used stsci.tools to parse the input and feed that to the executables. In python, I think the better sequence is to make a list of images to be processed and call the executable on that list. I think the docs are misleading and they should be updated; The code was probably made explicit with the stsci.tools call to move people towards that direction.

[philhodge]

Do wfc3tools use hstcal? The code in calwf3 calls c_imtopen, which accepts a string that can include multiple file names, separated by a blank or comma.

[sosey]

I'm fuzzy, but wfc3tools is calling the individual executables. The c_imtopen is called higher up in the C pipeline to do the parsing of multiple filenames and feeds that down, or creates and association file (again, I'd have to go look at the code again to verify, but that's what is in my head)

[sosey]

It's possible calwf3.e does something different that the later stage executables since it's called high up

[jamienoss]

I think wrappers (of this kind) should be thin, sometimes even "wafer thin". So, if the underlying exec accepts a list, the list should be passed through so as to not alter functionality symmetry, now or in the future. I know there is a mindset to treat the python tools as the primary entity, especially by INS who use them, but they're not and nor should they be.

Now if the underlying exec doesn't accept lists then we could add this functionality to the Python wrappers. I would still argue that this functionality should go in the C code base, but either way that's not covered by this issue - but instead this one spacetelescope/hstcal#179.

[sosey]

I believe that the calwf3 exec only accepts 1 filename, and it expects multiple observations to come in the form of an asn table, this was historically mandated by the project. The subroutines may accept more, those have been extremely rarely used by anyone even inside the cal teams. My memory is the change here was made for consistency of calling the routines in this package. wfc3tools is meant to be used as the pythonic interface into the C executables, not the primary interface.

If the team has decided they'd rather go back to using lists of files as input to the subroutines from the wrapper, make sure that the docs are explicit in the calling differences between calwf3 and the subs. Functionality to accept lists of files doesn't need to be added to the wrappers, users should call the executable from their list of files.

[philhodge]

The one component (at least for STIS) where it has been important for the calxxx function to accept a list (actually a blank or comma-separated-value string) is calstis2, cosmic-ray rejection. For WFC3, calwf3/wf3ccd/mainccd.c calls c_imtopen with that string as input.

[sosey]

For wfc3, the IR channel does cosmic ray rejection inside the ramp for a single image. In UVIS, the CR rejection is done with drizzle after exiting calwf3. I remember the calwf3 executable calling a separate routine (procccd or procir) to run the subroutines directly, meaning the executables for the subroutines have a different entrance, through their main, and it's only that entrance that calls c_imtopen. For example, you can only access the list input functionality if you call wf3ccd.e from the shell standalone, which was built from its main, and give it the files it expects, it then loops the call to wf3ccd from the list. calwf3 just calls WF3ccd on the single file.

[philhodge]

For STIS it's often used to do cosmic-ray rejection on a collection of bias or dark files, e.g. 40 files.

[sosey]

I think the teams prefer to do cosmic-ray rejection in different ways. However, they can currently call the subroutines to do regular calibration. None of the calwf3 subroutines perform cosmic-ray rejection in uvis.

[mdlpstsci]

Thanks to folks for all the historical perspective! Indeed, calwf3.e will only accept a single filename in contrast to some (would need to check them all) of the sub-component C executables which will accept a CSV string. In this specific case a user wished to use wf3rej.py to process a CSV string or alternatively an ASN, but neither option worked. The wf3rej.py routine should be able to handle the CSV as the underlying wf3rej.e allows this. As noted, the documentation for the tools needs to be updated to reconcile the descriptions with the actual state of the software.

[mdlpstsci]

Updated wf3rej.py on my private Git branch (issue22_inputlist) and offered this version for testing to the internal user. The response on 05 July 2018:
Hi Michele,
I just wanted to let you know that Nate was able to use your fixes successfully! If any questions come up in the future I’ll let you know.
Thanks,
Catherine

I need to follow up with the team to determine how they would like the tools to work.

[mdlpstsci]

Spoke to Deputy Lead of WFC3 and asked her to read over this issue to understand the general topic. If necessary, the issue can be raised at the next team meeting in order to determine if the current behavior should remain as it is or should be reconciled to the behavior of the C tools.

[nmiles2718]

Is there any update on how this issue will be handled? I rely heavily on the ability to process an input list of images with wf3rej. For my analysis I need the ability to run the CR rejection on a list of darks. Every time I try to process WFC3 data, I am forced to go through the process of cloning @mdlpstsci fork and building from source on the branch that contains the fix. This is a huge roadblock for two issues:

1. Reproducibility! If someone else would like to reproduce my analysis or generate the same data I have, they will fail miserably for UVIS dat because of this error. The documentation states that a python list of files should be able to be processed, yet the error raised contradicts that. The UVIS dataset is invaluable for a cosmic ray analysis as it provides increased coverage for Solar Cycle 24.

2. If I am forced to ignore all documentation and build my own association files for sets of darks to perform the cosmic ray rejection, I can no longer simply download the FLTs that have been bias-corrected and run CR rejection on those images. Instead I am relegated to download all of the RAW files and process them from scratch. This has a huge impact on the cloud functionality of the analysis pipeline because now I have to download all the reference files as well. At the present moment there is no clear or obvious way (to my knowledge at least) to download reference files via astroquery and so this makes any solution I come up subject to failure in the future as this problem will likely be addressed later on. This means the analysis can no longer be run on AWS which is one of the main highlights of the pipeline.

[mdlpstsci]

I will email and talk with the Deputy Lead of WFC3 today or as soon as I can get a few minutes with her.

[mdlpstsci]

I got a response that Sylvia will review the issue this week so she and I can talk in an informed way.

[nmiles2718]

Ok, great! Thanks @mdlpstsci!

[mdlpstsci]

After consultation with @SMBaggett (who also discussed this with HSTMO), this issue has been given the permission to proceed. It is understood this issue may also be applicable to some, if not all, of the other WFC3 tools.