fraserw / trippy

Python based Trailed Source Photometry
GNU General Public License v3.0
20 stars 13 forks source link

Bugs in ``pillPhot`` #24

Open Mikea1985 opened 6 years ago

Mikea1985 commented 6 years ago

I found three bugs in pill.pillphot. I'll post them here together, just in case they're related, but I don't think they are; also, that way I can refer to the same figures. Bug 1+3 only happen when using an array of apertures rather than just a single value. Bug 1+2 only occur when the user zooms in to select a custom background selection; if the full sky-area is used (window closed without any zooming) neither error occurs. This code ( https://www.dropbox.com/s/wx1ga6d2cnf87xq/tst.py?dl=0 ) and this image ( https://www.dropbox.com/s/bnptp6520m0xp7k/mrgN20131205S0190.fits?dl=0 ) reproduce the bug 1+2 upon zooming and bug 3 upon changing the angle to a value other than 0.

Bug 1): Sometimes when I zoom, values appear on the histogram that didn't exist before. :-0 Exhibit A: Screenshot before zooming image_display1 Exhibit B: Screenshot after zooming a bit; where did those values at >10,000 come from?!? image_display2 Exhibit C: Screenshot after zooming a bunch more. The >10,000 values appear to come from (near?) the trimmed pixels. Maybe the there is a bug in the trimming? image_display3 Exhibit D: Screenshot from editing the code to have trimBGHighPix=30.0, rather than =3 as in the attached script. The high values do indeed appear to correspond to the trimmed pixels, which appear to be a cosmic ray trail. image_display4 In conclusion: While the trimming is still being plotted on the 2D image-cutout when we zoom, it appears that the pixel-values used for the histogram no longer gets trimmed after having zoomed. It's unclear whether this affects the calculated average background, but there's a good chance it does.

Bug 2): When I zoom, using a numpy.array of aperture sizes no longer works. In the above case, Exhibit A results in magnitudes [ 24.50516588 24.32180994 24.26175012 24.21275257 24.14931926 24.13903206 24.08707976], whereas Exhibit B results in 16.9700653234; a single value rather than a value per aperture, and to top it all off, it's an obviously wrong value.

Bug 3): If I change the angle in the script mentioned above to any value other than 0, +90 or -90, I get the following error before anything is even plotted:

/home/mikea/.local/lib/python2.7/site-packages/trippy/trippy_utils.py:62: RuntimeWarning: invalid value encountered in double_scalars self.m=(p2[1]-p1[1])/(p2[0]-p1[0]) /home/mikea/.local/lib/python2.7/site-packages/trippy/trippy_utils.py:62: RuntimeWarning: divide by zero encountered in double_scalars self.m=(p2[1]-p1[1])/(p2[0]-p1[0]) Traceback (most recent call last): File "tst.py", line 24, in zscale=False) File "/home/mikea/.local/lib/python2.7/site-packages/trippy/pill.py", line 310, in call xxx = mx0-num.cos(a)radiusself.repFact ValueError: operands could not be broadcast together with shapes (25,) (7,)

I haven't had a chance to look at the code to identify the problem yet. I'll have a look at fixing this tomorrow (at the very least bug 2), since I need it to work ASAP.

fraserw commented 6 years ago

1 was known and forgotten about. Is due to an auto-bad-pixel removal not propagating after the selecion. Should be easy to fix.

2 is new.

3 is scary!

Thanks dude. Will have to look at these soon

Wes

On Jun 11, 2018, at 5:44 PM, Mike Alexandersen notifications@github.com wrote:

I found three bugs in pill.pillphot. I'll post them here together, just in case they're related, but I don't think they are; also, that way I can refer to the same figures. Bug 1+3 only happen when using an array of apertures rather than just a single value. Bug 1+2 only occur when the user zooms in to select a custom background selection; if the full sky-area is used (window closed without any zooming) neither error occurs. This code ( https://www.dropbox.com/s/wx1ga6d2cnf87xq/tst.py?dl=0 https://www.dropbox.com/s/wx1ga6d2cnf87xq/tst.py?dl=0 ) and this image ( https://www.dropbox.com/s/bnptp6520m0xp7k/mrgN20131205S0190.fits?dl=0 https://www.dropbox.com/s/bnptp6520m0xp7k/mrgN20131205S0190.fits?dl=0 ) reproduce the bug 1+2 upon zooming and bug 3 upon changing the angle to a value other than 0.

Bug 1): Sometimes when I zoom, values appear on the histogram that didn't exist before. :-0 Exhibit A: Screenshot before zooming https://user-images.githubusercontent.com/14901128/41242440-1bca7340-6dd2-11e8-825a-912dbe36966e.png Exhibit B: Screenshot after zooming a bit; where did those values at >10,000 come from?!? https://user-images.githubusercontent.com/14901128/41242969-976c006c-6dd3-11e8-82f8-b27711ef7164.png Exhibit C: Screenshot after zooming a bunch more. The >10,000 values appear to come from (near?) the trimmed pixels. Maybe the there is a bug in the trimming? https://user-images.githubusercontent.com/14901128/41243111-f0e785c6-6dd3-11e8-8866-a7b0cbf38d60.png Exhibit D: Screenshot from editing the code to have trimBGHighPix=30.0, rather than =3 as in the attached script. The high values do indeed appear to correspond to the trimmed pixels, which appear to be a cosmic ray trail. https://user-images.githubusercontent.com/14901128/41243266-48aa96ea-6dd4-11e8-88a9-3a8ce0c884ce.png In conclusion: While the trimming is still being plotted on the 2D image-cutout when we zoom, it appears that the pixel-values used for the histogram no longer gets trimmed after having zoomed. It's unclear whether this affects the calculated average background, but there's a good chance it does.

Bug 2): When I zoom, using a numpy.array of aperture sizes no longer works. In the above case, Exhibit A results in magnitudes [ 24.50516588 24.32180994 24.26175012 24.21275257 24.14931926 24.13903206 24.08707976], whereas Exhibit B results in 16.9700653234; a single value rather than a value per aperture, and to top it all off, it's an obviously wrong value.

Bug 3): If I change the angle in the script mentioned above to any value other than 0, +90 or -90, I get the following error before anything is even plotted:

/home/mikea/.local/lib/python2.7/site-packages/trippy/trippy_utils.py:62: RuntimeWarning: invalid value encountered in double_scalars self.m=(p2[1]-p1[1])/(p2[0]-p1[0]) /home/mikea/.local/lib/python2.7/site-packages/trippy/trippy_utils.py:62: RuntimeWarning: divide by zero encountered in double_scalars self.m=(p2[1]-p1[1])/(p2[0]-p1[0]) Traceback (most recent call last): File "tst.py", line 24, in zscale=False) File "/home/mikea/.local/lib/python2.7/site-packages/trippy/pill.py", line 310, in call xxx = mx0-num.cos(a)radiusself.repFact ValueError: operands could not be broadcast together with shapes (25,) (7,)

I haven't had a chance to look at the code to identify the problem yet. I'll have a look at fixing this tomorrow (at the very least bug 2), since I need it to work ASAP.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/fraserw/trippy/issues/24, or mute the thread https://github.com/notifications/unsubscribe-auth/AGKcvrCHfo1TegaSZd_AESU98_EG-DQsks5t7p5SgaJpZM4UjCAi.

Mikea1985 commented 6 years ago

Bug 2 fixed. I'll make a pull request when I get the other two.

Yeah, I suspected bug 1 might be known, possibly even intentional (although if intentional, then I felt it was a bug that the trim was still being shown in the 2d-plot).

fraserw commented 6 years ago

No #1 wasn’t intentional. Just not bothered to fix it in the last 2… years… lol

Thanks For snagging #2

On Jun 12, 2018, at 4:05 AM, Mike Alexandersen notifications@github.com wrote:

Bug 2 fixed. I'll make a pull request when I get the other two.

Yeah, I suspected bug 1 might be known, possibly even intentional (although if intentional, then I felt it was a bug that the trim was still being shown in the 2d-plot).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/fraserw/trippy/issues/24#issuecomment-396451119, or mute the thread https://github.com/notifications/unsubscribe-auth/AGKcvkfLnLk8zYoJ9Ab8Zs_gzdu886J5ks5t7y_hgaJpZM4UjCAi.

Mikea1985 commented 6 years ago

In the process of fixing 3, I also noticed and fixed bug 4: The white outline of the aperture is only drawn for pill-apertures (no circles) and only if the angle is not 0 or +/-90.

I'm having trouble wrapping my head around fixing bug 1, since I don't really understand how you did the trimming/masking in the first place.

fraserw commented 6 years ago

Fix to bug 1 is something like, maintaining the bad pixel mask, which is initially identified using a np.where call. Then, when a zoom is done, apply the zoom to the where output as well.

On Jun 12, 2018, at 9:16 AM, Mike Alexandersen notifications@github.com wrote:

In the process of fixing 3, I also noticed and fixed bug 4: The white outline of the aperture is only drawn for pill-apertures (no circles) and only if the angle is not 0 or +/-90.

I'm having trouble wrapping my head around fixing bug 1...

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/fraserw/trippy/issues/24#issuecomment-396505882, or mute the thread https://github.com/notifications/unsubscribe-auth/AGKcvo8gTUlWEW45GYDAmAXJ62rLw8V-ks5t73jYgaJpZM4UjCAi.

Mikea1985 commented 6 years ago

Sure, sure, but I'm definitely not understanding the code correctly. I have (possibly by dumb luck) managed to make the masking carry through for the photometry, but not for the histogram :-S. I really don't understand how the histogram is created and updated; it's a bit too object-oriented for my current understanding of python objects.

fraserw commented 6 years ago

OK. I can handle that one. Don’t worry about it!

Wes

On Jun 13, 2018, at 3:09 AM, Mike Alexandersen notifications@github.com wrote:

Sure, sure, but I'm definitely not understanding the code correctly. I have (possibly by dumb luck) managed to make the masking carry through for the photometry, but not for the histogram :-S. I really don't understand how the histogram is created and updated; it's a bit too object-oriented for my current understanding of python objects.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/fraserw/trippy/issues/24#issuecomment-396790699, or mute the thread https://github.com/notifications/unsubscribe-auth/AGKcvpt31hU0WzB6FzxqGzmVnWTbmLe_ks5t8HRFgaJpZM4UjCAi.

fraserw commented 4 years ago

Just checking in - is this issue solved now in current versions?