Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Star catalog centroids #1

Open
dkirkby opened this issue Aug 28, 2013 · 7 comments
Open

Star catalog centroids #1

dkirkby opened this issue Aug 28, 2013 · 7 comments

Comments

@dkirkby
Copy link

dkirkby commented Aug 28, 2013

According to the guide:

The first star in the catalog is precisely centered within a pixel, and the others have
random sub-pixel offsets.

However, looking at starfield_image-100-0.fits, the first star is actually centered on the stamp which, because of the even stamp size, is actually not centered in a pixel. I think the star images are fine as is, but it would be good to clarify how they are centered in the guide.

A related issue is that the corresponding catalog gives the star center as (23,23) but with the usual convention that the bottom-left corner is (1,1), the center is actually on the corner between (24,24) and (25,25). Presumably the convention here is actually 0 indexed, so that would be good to highlight (or perhaps change?)

@rmandelb
Copy link
Collaborator

Hi David -

I think I agree with you about changing the documentation regarding the star centroid, rather than changing the images. Let me just check with you: I found this incorrect statement located on
https://github.com/barnabytprowe/great3-public/wiki/Challenge-dataset-format
and in the handbook. Did you see it anywhere else? (I'm not asking you to go searching, but if those places are not where you saw it, please let me know so I can fix it everywhere.)

Regarding the pixel convention, I'm on the fence about documenting vs. changing. @barnabytprowe , @rmjarvis , @TallJimbo, any opinions?

@dkirkby
Copy link
Author

dkirkby commented Aug 28, 2013

No, I haven't seen this anywhere else.

@TallJimbo
Copy link

The FITS and FORTRAN conventions are to start at (1,1), but the C/C++ and Python (and LSST, FWIW) conventions are to start at (0,0), and that's essentially what we've adopted in GalSim. While we could change it in the GREAT3 public catalogs without changing all the GalSim code, I think consistency with GalSim is highly desirable, and I'd recommend that we just document the (0,0) convention rather than switch to (1,1). It's also almost certain that our example analysis script will be written in Python, and we'd have to do a lot of adding/subtracting by 1 to make that appear to work in (1,1)-origin coordinates.

@dkirkby
Copy link
Author

dkirkby commented Aug 28, 2013

GalSim internally uses 0-offset (and x-y transposed) numpy arrays but externally uses indexing from 1 in at least a few places. For example:

galsim.ImageD(48,48).bounds
BoundsI(xmin=1, xmax=48, ymin=1, ymax=48)

I think its ok to stick with 0-indexing, but since all the data is distributed in FITS files, there is some potential for confusion when people compare with coordinates in DS9, etc.

For the record, there was some earlier discussion of this at GalSim-developers/GalSim#380 (comment)

@TallJimbo
Copy link

Hmm, I wasn't aware of this internal inconsistency. I'd still like to stick with the (0,0) convention, but it sounds like it'd be worthwhile doing a sweep through GalSim and fixing this and any other places where we use a (1,1) convention at some point. Maybe not before 1.0, though.

@rmjarvis
Copy link
Collaborator

Hmm, I wasn't aware of this internal inconsistency. I'd still like to stick with the (0,0) convention, but it sounds like it'd be worthwhile doing a sweep through GalSim and fixing this and any other places where we use a (1,1) convention at some point. Maybe not before 1.0, though.

For the record, in the discussion David referenced, the default (1,1) indexing was considered a desired feature, rather than a bug. You can change this with image.setOrigin(0,0) in python or index_convention=0 in config.

As for what convention to use for the positions in the catalogs we distribute for Great3, I don't have a strong preference so long as it is documented what we mean by these numbers.

Likewise that the first star is not centered on a pixel, but rather centered on the postage stamp which means the centroid is at the corners of 4 pixels. I think it is fine to just change the documentation about this rather than recenter these stars.

@rmandelb
Copy link
Collaborator

For now I've changed the documentation on the wiki, so as to ensure the documentation is consistent with the images + catalogs.

We could easily change conventions, but (a) I don't care that the first star is not centered on a pixel, and nobody else on this thread seems to care either (my impression was that David only cared about correctness of documentation), so let's not change that unless someone has a legitimate objection; and (b) for the catalog pixel conventions, for now it helps that the documentation is more explicit (it already said pixel coordinates were zero-indexed, though now it notes that this explicitly differs from FITS conventions, and clarifies "For example, "23" means it is at the center of the first postage stamp, with 48 pixels on a side, so in a 1-indexed convention, the centroid would be at the edge between pixels 24 and 25 along the x direction."). However, I am more inclined to change this because of the point David made about comparing with 1-indexed pixel values when viewing in some typical FITS-reader like ds9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants