Inconsistency in command-line interface related to mdriztab=True

[stscijgbot-hstdp]

Inconsistency in command-line interface related to mdriztab=True

[stscijgbot-hstdp]

Comment by Steve Goldman on JIRA:

This issue was recently raised again in a helpdesk ticket INC0198393. It's unclear when the issue arose, or whether this has always been a problem. Most of the related files don't seem to have been updated in at least the last 5 years. 

The critical code is line 131 of processInput.py where user values are merged with mdriztab default values. Since the mdriz file (which hasn't been updated recently) has a default value for final_rot of {}None{}, the user value is being replaced by {}None{}. 

There is a comment in the code there stating:

    # interpret MDRIZTAB, if specified, and update configObj accordingly
    # This can be done here because MDRIZTAB does not include values for
    # input, output, or updatewcs.```
 

[mcara]

Maybe @mackjenn can remember how MDRIZTAB was intended to be used. I very vaguely recall some discussions around MDRIZTAB (> 5 years ago). It could be that the code actually works as intended (MDRIZTAB overrides user input - it could be what INS wanted) but MDRIZTAB should not have had custom WCS parameters: obviously these are not intended to be used in the pipeline. So, either the logic in the code is buggy or the MDRIZTAB file contains fields that it was not supposed to have.

For now, users can load MDRIZTAB and delete parameters that they supply as input to astrodrizzle:

from drizzlepac.mdzhandler import getMdriztabParameters
mdrztab = getMdriztabParameters(['file_name1_flc.fits'])
param_to_del = [
    'final_wcs',
    'final_outnx',
    'final_outny'
]
for k in param_to_del:
    del mdrztab[k]
AstroDrizzle(
    input,
    mdriztab=False,
    configobj=mdrztab,
    final_wcs=True,
    final_outnx=1111,
    final_outny=2222
)

[s-goldman]

Thanks Mihai, this is really helpful. Unfortunately, it looks like deleting the parameters results in those parameters no longer being recognized as viable user inputs. That being said, I think I can find a fix along these lines using getMdriztabParameters. Thanks!

[s-goldman]

A possible solution is something like the following, however, there seem to be some issues with the result. Still investigating.

from stsci.tools import teal
from drizzlepac.mdzhandler import getMdriztabParameters

configObj =  teal.load("astrodrizzle")
configObj["mdrztab"] = getMdriztabParameters(["j96001cdq_flt.fits"])

input_flts = [
    "j96001cfq_flt.fits",
    "j96001cdq_flt.fits",
    "j96001cgq_flt.fits",
    "j96001ciq_flt.fits",
    "j96001clq_flt.fits",
    "j96001cjq_flt.fits",
    "j96001cmq_flt.fits",
    "j96001coq_flt.fits",
]

configObj["input"] = input_flts
configObj["output"] = "SUMMED_test"

configObj["mdrztab"]["driz_cr_corr"] = True
configObj["mdrztab"]["final_refimage"] = "hlsp_hlf_hst_wfc3-60mas_goodss_f275w_v2.0_conv_sci.fits"
configObj["mdrztab"]["final_rot"] = -35
configObj["mdrztab"]["final_outnx"] = 10000
configObj["mdrztab"]["final_outny"] = 10000

configObj["STEP 7a: CUSTOM WCS FOR FINAL OUTPUT"]["final_wcs"] = True


astrodrizzle.AstroDrizzle(
    configobj=configObj,
)

[stscijgbot-hstdp]

Comment by Steve Goldman on JIRA:

After discussing this with Warren, it looks like the desired outcome is still being produced. The mdriztab=True option is resulting in the best pipeline parameters being passed to the configobj. The problem is that, while the mdriztab pipeline parameters should supersede the user's parameters, there should be a warning to the user that this is happening. 

[mcara]

The mdriztab=True option is resulting in the best pipeline parameters being passed to the configobj.

I have to disagree with this: the best parameters are the ones that user wants. And those seem to be ignored. For the very least there has to be a workaround developed for the users.

[s-goldman]

The mdriztab=True option is resulting in the best pipeline parameters being passed to the configobj.

I have to disagree with this: the best parameters are the ones that user wants. And those seem to be ignored. For the very least there has to be a workaround developed for the users.

I think you're right. This isn't the way it SHOULD work. I should have written above that this was the "previously" desired outcome. That being said, I'm not sure how common of an issue this is, and whether it's worth changing the way that the configuration parameters are merged. I will ping some of the other drizzling folks to see if the juice is worth the squeeze.

[mcara]

Most likely users do not even realize that their options are ignored and simply move on with processing. Pipeline is probably using mdriztab exclusively. So, allowing users to overwrite some of the parameters loaded from mdriztab would be the right thing to do, IMO.

And if you want to keep the existing behavior, I think that the warning mentioned in a previous message should be upgraded to an exception to force users remove their custom parameters: mdriztab must be used exclusively; no user parameters allowed. + provide an example with a workaround.

[s-goldman]

I totally agree. Good call 👍

[mackjenn]

Chiming in here... I agree with Mihai. The desired behavior is for the user to be able to use the MDRIZTAB default values as a starting point and then allow for any changes to those parameters from the command line.

For example, final_rot=INDEF in the MDRIZTAB, but you should be able to override this by setting final_rot=0 to drizzle north up, while retaining the other recommended values for each instrument.

astrodrizzle.Astrodrizzle('i*flt.fits', mdriztab=True, final_wcs=True, final_rot=0)

[stscijgbot-hstdp]

Comment by Michele De La Pena on JIRA:

As indicated, messages need to issued so the user is informed.

I will note that the logic here is not typical though there may be other issues at play. Under most circumstances, any values supplied on the command line as a direct parameter or as part of a file which is a direct parameter to a task should supersede any configuration values read by default by the task.

At the least this needs to be well-documented. We can talk about this, but perhaps it should just be fixed according to expectation.

[stscijgbot-hstdp]

Comment by Steve Goldman on JIRA:

Michele De La Pena After some discussions elsewhere including the github issue 1739, I think it was unanimous by the instrument scientists (and Mihai) that the user inputs should supersede the mdriztab defaults. I have a PR that should do the trick, and I am testing that now. 

[stscijgbot-hstdp]

Comment by Steve Goldman on JIRA:

closed by #⁠1774