bugGNU Astronomy Utilities - Bugs: bug #48978, Bad coordinates in polygon mode do...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #48978: Bad coordinates in polygon mode do not abort

Submitter:  Mohammad Akhlaghi <makhlaghi>
Submitted:  Fri 02 Sep 2016 05:21:34 PM UTC
   
 
Category:  Crop Severity:  4 - Important
Item Group:  Output not reasonable Status:  Fixed
Privacy:  Public Assigned to:  vvm
Open/Closed:  Closed

Tue 03 Jan 2017 04:24:54 PM UTC, comment #3: 

Thanks a lot Vladimir, I also checked with a few other bad numbers, and it works. Thanks also for the very complete and thorough commit message. It has been pushed to the main repo.

I just added some comments above your correction in the code, so the reader can get an idea of the purpose of this check without having to look up the history or mentally building scenarios. I also added some blank lines in the commit message for fitting better with the other commits.

The issue you raised about the negative number was interesting. Its not a bug, ImageCrop was working! Let me explain: In the FITS standard, we define the pixel resolution (in units of degree/pixel) using the `PC1_1' and `PC2_2' header keywords (when the image is aligned with the celestial grid). You can see them with Gnuastro's Header program:


$ astheader tests/mkprofcat1.fits
...
PC1_1   = -1.66666666245874E-06 / Coordinate transformation matrix element
PC2_2   =  1.66666666245874E-06 / Coordinate transformation matrix element
...


This says that each pixel is -1.66666666245874E-06, or that each degree is 600000 pixels (this is roughly the Hubble Space Telescope resolution). So by changing the polygon vertice from `1.0013217' to `-1.0013217', you have practically asked for the vertice to be 2 degrees below what it was meant to be and the image would be more than one million pixels along one dimension.

To confirm this, I done this change (and added the `--keepblankcenter' option, since the center will be blank and ImageCrop will delete it by default) and waited a little. On my system after about 30 seconds, the final image was made. It was 4.6GB and had a size of 960 x 1202018 pixels!

So fortunately this wasn't a bug, but you did raise an important point: A sanity check can be defined when we are in WCS mode (`p->wcsmode==1'), just before adding the coordinate to the list (with `gal_linkedlist_add_to_tdll'). In general, the right ascension (or first coordinate) should be between `0<=RA<360' and the declination should be between `-90<Dec<90'. Since you raised this limits issue could you implement this small check and post it as a task?

Mohammad Akhlaghi <makhlaghi>
Group administrator
Mon 02 Jan 2017 08:25:06 PM UTC, comment #2: 

I've tested the patch for a while. it looked good for catching extra characters as you wrote in your post (extra dot) and cases like putting accidentally editing in VIM a bash script (e.g, sometimes I type extra 'i' when changing VIM mode: "polygon=1.01i:2.0").

But I found another trouble. I am not sure if it is a job for this patch (to make it more strict) or it is better to create a new issue.
When I used a negative valid floating point number, the parser said it is fine. But the test hung up - I had to press Ctrl+C to break 'make check' running. I tried a few times with different negative numbers and it always hangs up. Is it OK to use negative numbers in polygon point list or the parser must show a error?

Vladimir Markelov <vvm>
Mon 02 Jan 2017 08:19:33 PM UTC, comment #1: 

Hi! I'm back with a small patch for parsing polygon point list. I assumed that the polygon point list is always a list of floating point numbers separated with ',' and ':'.

I made a patch and describe my way in the commit message. Please, see the patch at:
https://github.com/VladimirMarkelov/gnuastro-vvm/commit/179d5eb60cddc776a11184dc224bdf644c2253d8


Vladimir Markelov <vvm>
Fri 02 Sep 2016 05:21:34 PM UTC, original submission:  

In the values to the `--polygon' option, when a coordinate has two `.'s in it, ImageCrop will not halt with an error. Instead, it will continue and not give any error at all.

To reproduce this bug, please run this command in the build directory after running `make check' (this is actually taken from `tests/imgcrop/wcspolygon.sh', with a minor modification):


./src/imgcrop/astimgcrop ./tests/mkprofcat*.fits --wcsmode --zeroisnotblank --output=bugcheck.fits --polygon=0.99980497,1.0001967:0.998378,1.0012267:0.9999766,1.00132.17


Note how the last coordinate in the values to polygon is `1.00132.17', it should have been `1.0013217'. Such errors can happen when giving the coordinates by hand. Ideally ImageCrop should recognize this and abort, but it doesn't.

The problem can most probably be fixed within the `polygonparser' function of `src/imgcrop/crop.c' which is in charge of parsing the polygon values and is called within ImageCrop's `ui.c'. We should set it to print an error message an abort in such situations. I can't fix it right now, but it shouldn't be hard, so if you can do it sooner, please feel free to post the fix ;-).

Mohammad Akhlaghi <makhlaghi>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by vvm (Posted a comment)
  • -email is unavailable- added by makhlaghi (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-01-03 makhlaghi StatusIn Progress Fixed
        Assigned toNone vvm
        Open/ClosedOpen Closed
    2017-01-02 vvm StatusConfirmed In Progress

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code