Feature/assoc world coord

astrorama / SourceXtractorPlusPlus

SourceXtractor++, the next generation SExtractor

https://astrorama.github.io/SourceXtractorPlusPlus/

GNU Lesser General Public License v3.0

72 stars 9 forks source link

Feature/assoc world coord #488

Closed marcschefer closed 2 years ago

marcschefer commented 2 years ago

New parameter assoc-coord-type that can be set to WORLD to use world coordinates in the assoc catalog (default remains PIXEL).

This should be compatible with MEF detection images.

mkuemmel commented 2 years ago

I tested it and it works which is great!

Two comments:

please add the unit of --assoc-radius in the help text. I expected it to be [arcsec] for WORLD, but it is still [pix] independent of the coordinate type, and that should be clear!
the columns given in assoc-copy must be of type float, string and even int columns are not accepted. I think the problem is when writing the table to FITS the copy columns end up in in a vector column and that can not have a mixed type. When copying an int or string the error is quite generic 2022-06-30T14:24:28CEST SourceXtractor FATAL : Can't either open or read assoc catalog: ./point_like.hsc_r2.ascii. I would put a comment in the help that only float columns are accepted and then we can see. If people complain we can change that still.

marcschefer commented 2 years ago

the columns given in assoc-copy must be of type float, string and even int columns are not accepted.

Int columns should be accepted, there is code for that so maybe there is a bug. The result will always be float as we can specify an operation to be done done all matching sources such as taking the sum or average. So non numeric columns make no sense.

mkuemmel commented 2 years ago

the columns given in assoc-copy must be of type float, string and even int columns are not accepted.

Int columns should be accepted, there is code for that so maybe there is a bug. The result will always be float as we can specify an operation to be done done all matching sources such as taking the sum or average. So non numeric columns make no sense.

I can confirm that an integer column now works. I am not sure what the operations on matching sources mean, but I am not an expert.

Shall we try to get #476 working as well? The failure is directly when reading in the FITS table, which should be in: https://github.com/astrorama/Alexandria/blob/cd2e728b0c10d0520af4ac3786bee4007f1eafc8/Table/src/lib/FitsReader.cpp#L46

I tried to debug but failed.

marcschefer commented 2 years ago

Shall we try to get #476 working as well?

Sure, I'm getting a bit confused as there were several issues but do I have the test data for this one already?

mkuemmel commented 2 years ago

Test data is available here: https://deepdip.iap.fr/#folder/62974b6b6c26b9d0d2c5e1d6

The script to run is test_assoc.sh. You need to change to acat=./point_like.hsc_r2.fits and maybe add in the SE++ command the ASSOC_TYPE (only to get meaningful results).

Alternatively you could send me your assoc list and I can convert to FITS easily.

ayllon commented 2 years ago

1506715 fixes the handling of the coordinates when they are float but we expect doubles (and also work with double => double). I get to run test_assoc.sh now end to end.

2022-07-07T10:03:27CEST SourceXtractor  INFO : total 2461 sources detected

@mkuemmel , the line where you mention he error comes from, did you try perhaps with a catch throw on gdb? Because CCfits throws internally a lot when parsing the headers of a FITS file, but that does not affect the success of reading the catalog.