Keck-DataReductionPipelines / KCWI_DRP

KCWI python DRP
BSD 3-Clause "New" or "Revised" License
8 stars 12 forks source link

Uncertainty not consistent in the icubes/icubed/icube #118

Closed zhuyunz closed 2 years ago

zhuyunz commented 2 years ago

Hi, I was wondering whether the 'uncert' in the cube stands for the sigma or the variance. By comparing the results from IDL and Python version, it seems to me that the uncertainty in the icube and icubed represents the sigma rather than the variance. However, FluxCalibrate.py treats it as the variance, so the final flux-calibrated uncertainty is not comparable to the one derived from the IDL version.

rrvaught commented 2 years ago

Hi, I have also noticed the same discrepancy. Comparing the cubes from IDL with Python, there is disagreement between the values when treating the Python either as sigma or uncertainty.

MNBrod commented 2 years ago

Could you give me a specific frame as an example (date and framenum are enough, I can pull it from there). My understanding IDL was using variance, and I know that the python one does. We will look at the IDL code to double check what that one does; since the old one isn't supported we don't know of the top of our head.

zhuyunz commented 2 years ago

Sure! You are welcome to use my data taken on 211002. Frame#51 is a science frame and #76 is a std frame.

I reduced this data set using both IDL and Python DRP. From my comparison, 'uncert' dimensions in icube and icubed in Python version are consistent with the square root of IDL variance, so they should be the sigma. The conversion in the FluxCalibrate.py mistakenly treat the sigma as variance. Once I modified the flux calibration in the code, I could get the consistent results for Python and IDL.

MNBrod commented 2 years ago

Sorry this took so long to get to this, I've only just gotten some free time. You are definitely correct! FluxCalibrate swaps to variance unexpectedly. I will get this fixed and pushed with the next update to the pipeline, which will be coming in about a week.

MNBrod commented 2 years ago

Fixed with #128, will be pushed to pip later today.