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

Inconsistent treatment of AperturePhotometry <--> AutoPhotometry #370

Open mkuemmel opened 3 years ago

mkuemmel commented 3 years ago

The output-properties AperturePhotometry and AutoPhotometry are treated differently:

I would be more consistent to either making measurement images mandatory also for AutoPhotometry or (re-)introducing using the detection image for AperturePhotometry when there are no measurement images (which would require introducing an aperture parameter).

The confusion extends also to the checkimages. In my example with only the detection image and giving: --check-image-aperture=aper.fits --check-image-auto-aperture=autoaper.fits results in four images:

For both photometries there is one checkimage referring to the detection image and one referring to the measurement images. This is rather confusing.

ayllon commented 3 years ago

I would be more consistent to either making measurement images mandatory also for AutoPhotometry or (re-)introducing using the detection image for AperturePhotometry when there are no measurement images (which would require introducing an aperture parameter).

Maybe I would rather add back the aperture config to the initial config file. It seems more usable. However, it can become confusing if things are specified twice (python and config file). I would rather fail quick giving a descriptive error message in that case. i.e. Aperture photometry can not be configured both globally and per image. What do you think?

For both photometries there is one checkimage referring to the detection image and one referring to the measurement images. This is rather confusing.

The detail here is that when there is no Python config file, the detection image is kind of "duplicated". Internally there is still independent detection and a measurement frame.

Since units are generally given in detection-frame pixels, that's why check-images are generated twice: the aperture in your detection frame, and the aperture projected into the measurement frame. Which happens to be the identity projection.

This is the explanation. It may be worth adding a method that can be checked so the properties know to generate things only once (i.e. Frame::isSameAsDetection or the like)

mkuemmel commented 3 years ago

I would be more consistent to either making measurement images mandatory also for AutoPhotometry or (re-)introducing using the detection image for AperturePhotometry when there are no measurement images (which would require introducing an aperture parameter).

Maybe I would rather add back the aperture config to the initial config file. It seems more usable. However, it can become confusing if things are specified twice (python and config file). I would rather fail quick giving a descriptive error message in that case. i.e. Aperture photometry can not be configured both globally and per image. What do you think?

Adding back aperture config to the ASCII config is certainly the better solution. Many users do not need to work with measurement images but still are interesting having aperture photometry.

mkuemmel commented 3 years ago

Aperture photometry on the detection image is possible. The name for the aperture parameter and the output-properties point to the detection image.