pypeit / PypeIt

The Python Spectroscopic Data Reduction Pipeline
BSD 3-Clause "New" or "Revised" License
163 stars 104 forks source link

Major bug in pypeit parset merge? #736

Closed jhennawi closed 5 years ago

jhennawi commented 5 years ago

The PypeItPar.from_cfg_lines does not appear to be merging parsets properly, between spectrograph defaults and .pypeit file. You can reproduce bug as follows:

1) Put in a an embed or debugger entry in line 95 in pypeit after the line: self.par = PypeItPar.from_cfg_lines(cfg_lines=spectrograph_cfg_lines, merge_with=cfg_lines) 2) Edit the following dev suite PypeIt file: ~/pypeit_files/vlt_xshooter_vis_A.pypeit

Add the line

[calibrations] [[arcframe]] bias = skip

The merge is not working correctly.

Thanks!

kbwestfall commented 5 years ago

Actually, it's just because the bias keyword is part of the process subgroup in arcframe. So it should be:

# User-defined execution parameters
[rdx]
spectrograph = vlt_xshooter_vis
[calibrations]
    [[arcframe]]
        number = 1
        [[[process]]]
            bias = as_available

in your pypeit file. Then if I put a stop in the same place where you were just showing me:

(Pdb) self.par['calibrations']['arcframe']['process']
Parameter     Value         Default       Type        Callable
--------------------------------------------------------------
overscan      median        savgol        str         False
overscan_par  5, 65         5, 65         int, list   False
match         -1            -1            int, float  False
combine       weightmean    weightmean    str         False
satpix        reject        reject        str         False
sigrej        -1            20.0          int, float  False
n_lohi        0, 0          0, 0          list        False
sig_lohi      3.0, 3.0      3.0, 3.0      list        False
replace       maxnonsat     maxnonsat     str         False
lamaxiter     1             1             int         False
grow          1.5           1.5           int, float  False
rmcompact     True          True          bool        False
sigclip       4.5           4.5           int, float  False
sigfrac       0.3           0.3           int, float  False
objlim        3.0           3.0           int, float  False
bias          as_available  as_available  str         False

Let me know if that doesn't work.

kbwestfall commented 5 years ago

We probably want to have the code fault if a parameter is put on the wrong parameter set, but that might not be trivial. Leaving this open for now to remind us to look into this.

jhennawi commented 5 years ago

I think the code should crash if the brackets are incorrect.

jhennawi commented 5 years ago

Otherwise users think they changed a parameter but they did not. Without these we leave the potential for huge errors like with flexure or heliocentric on/off.

profxj commented 5 years ago

This is the same Issue as #701