search

spacetelescope / wfc3tools

Python tools for HST WFC3 data calibration and analysis
https://wfc3tools.readthedocs.io/en/latest/
Other
6 stars 19 forks source link

Pstack/Pstat rows and column passing is flipped #39

Closed Vb2341 closed 1 year ago

Vb2341 commented 5 years ago

In pstack, the input column and row parameters appear to be put in to the array indexing incorrectly. Since numpy's convention is row major order, this should be flipped. @mackjenn tested this and finds that pstack selects the wrong pixel (the pixel at the flipped coordinates) compared to doing a similar process with IRAF.

https://github.com/spacetelescope/wfc3tools/blob/6a11234645831e774cb6f3b23e18a14db815b9ae/wfc3tools/pstack.py#L45

mackjenn commented 5 years ago

Here's my test: imstat /grp/hst/wfc3a/GO_Links/12442/VisitBF/ibohbfb9q_ima.fits[1][591:591,508:508] fields=mean MEAN= 3.262

Running with Col=x, Row=y gives the wrong result (1.517): pstack('/grp/hst/wfc3a/GO_Links/12442/VisitBF/ibohbfb9q_ima.fits', column=590,row=507, units='rate') array([1.51687181, 1.47712266, 1.45100856, 1.39073086, 1.22206366, 0.97621757, 0.66732371, 0.62105507, 0.38288745, 0.25278419, 1.01287377, 0.37952417, 0.04332354, 2.44659662, 6.94167709,

  1. ]))

Running with Col=x, Row=y gives the correct result (3.262) pstack('/grp/hst/wfc3a/GO_Links/12442/VisitBF/ibohbfb9q_ima.fits', column=507,row=590, units='rate') array([3.26224589, 3.57222438, 4.02177429, 4.51260233, 4.99251795, 5.48985338, 5.96749544, 7.43224669, 7.0058403 , 7.26775551, 7.97073126, 4.3443408 , 6.16879797, 3.26105762, 3.74457192, 0. ]))

Vb2341 commented 5 years ago

In pstat, when a region is given, it doesn't say which order to supply the image section (slice) in. This doesn't explicitly say row and column like pstack, but since pstack does, it's worth keeping this consistent by either flipping how the arrays actually get indexed in pstack/pstat, OR by explicitly setting pstats inputs/documentation correctly. I would say the former is probably the better option when comparing this to a displayed image.

cshanahan1 commented 4 years ago

In addition to fixing this, we should make a separate input parameter for image slice instead of passing it in in the image name...

mackjenn commented 1 year ago

@mdlpstsci Hi Michele, This is a bug in pstat which we would like to fix or we can't use this tool in our user tutorials. Its not urgent, but I wanted to raise it again since it's been several years.

Can you open a PR for this work and add me and @acalamid to the ticket?

mdlpstsci commented 1 year ago

@mackjenn Since there is already an issue in GitHub, I can make a JIRA ticket for this work. I would not generate a GitHub PR until there is an actual code change. I have added you and Annalisa as watchers on HLA-912.

nden commented 1 year ago

Fixed in #75, #73