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

DM-34414: Add fields needed for deblender metrics to object tables #169

Merged
merged 2 commits into from Dec 6, 2023

Conversation

fred3m
Copy link
Contributor

@fred3m fred3m commented Dec 1, 2023

No description provided.

Copy link
Contributor

@taranu taranu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some copy-pasted descriptions that need to be fixed.

yml/hsc.yaml Outdated Show resolved Hide resolved
yml/hsc.yaml Outdated Show resolved Hide resolved
yml/hsc.yaml Outdated Show resolved Hide resolved
yml/hsc.yaml Outdated Show resolved Hide resolved
yml/hsc.yaml Outdated Show resolved Hide resolved
yml/imsim.yaml Outdated Show resolved Hide resolved
yml/imsim.yaml Outdated Show resolved Hide resolved
yml/imsim.yaml Outdated Show resolved Hide resolved
@fred3m fred3m force-pushed the tickets/DM-34414 branch 3 times, most recently from d0bebb7 to c6e2c72 Compare December 4, 2023 18:16
Copy link
Collaborator

@gpdf gpdf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the unit "pixel" (to fits:tunit:) where appropriate in the new attributes. I have indicated one example in an inline comment below.

I've also asked for confirmation that the "x" and "y" values are in fact intended to be integers.

(Assuming that I am interpreting the meaning of the column correctly.)

yml/hsc.yaml Outdated
datatype: int
description: x-coordinate of the peak after source detection
mysql:datatype: INTEGER
fits:tunit:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please supply the unit pixel here (assuming I'm understanding this correctly). Also: I note that this is declared as int, i.e., not supporting fractional-pixel positions. I don't know if that's what was intended, so can you just confirm that this is what you want?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is intended. These are not the centroids of the objects, they are the integer peak positions from the detection algorithm, which is useful for tracking detections across different versions of the science pipelines because they aren't dependent on measurements, deblending, etc.

@fred3m fred3m merged commit 0fee0e7 into main Dec 6, 2023
4 checks passed
@fred3m fred3m deleted the tickets/DM-34414 branch December 6, 2023 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants