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

Enable users to use different survey setups (pixel_scale, psf_fwhm) #202

Merged
merged 21 commits into from
Dec 18, 2023

Conversation

mr-superonion
Copy link
Collaborator

This PR enable users to use different survey setups. related to #188

@mr-superonion
Copy link
Collaborator Author

mr-superonion commented Dec 8, 2023

It is still a draft.

@mr-superonion mr-superonion requested review from esheldon and beckermr and removed request for esheldon December 9, 2023 09:29
@mr-superonion
Copy link
Collaborator Author

@esheldon, @beckermr , can you please review this PR? It enable users to use different survey's pixel scales and magnitude zero points.

@mr-superonion
Copy link
Collaborator Author

mr-superonion commented Dec 11, 2023

Hmm, I think I need more tests before merging... now the code can make simulations for two observations (HSC: left; DES: right) with the same input galaxies. But it seems that there is a shift between the input galaxies on the sky for these two sims.
image

@mr-superonion
Copy link
Collaborator Author

Let me add a test running the ML detection code and check the centroids have the same sky coords or not..

@mr-superonion
Copy link
Collaborator Author

mr-superonion commented Dec 12, 2023

I think now the sky coordinate matched between the two sims:
image
Right is HSC; left is DES.
I would say (hmm...) this is probably ready to merge...

Copy link
Collaborator

@esheldon esheldon left a comment

Choose a reason for hiding this comment

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

I like these changes generally.

However, it is really exposing limitations of the design choices we made. For example the Layout objects are nice, and probably we should have used them from the beginning. Then we would not need to pass around coadd_dims everywhere.

It may be we eventually want to do a restructuring around survey objects and layout objects. That would, for example, make it easy to simulate CCDs where coadd_dims is not relevant. Also the survey would carry around pixel scale etc.

descwl_shear_sims/shifts.py Outdated Show resolved Hide resolved
@mr-superonion
Copy link
Collaborator Author

mr-superonion commented Dec 12, 2023

make it easy to simulate CCDs where coadd_dims is not relevant.

Thanks!
Hmm, currently, we are using coadd_dims and coadd_pixel_scale, sky_orgin (not implemented, now it is fixed to (WORLD_ORIGIN)) passed to Layout object to define the galaxy distribution (taking flat-sky approximation). At the same time we are using coadd_dims and pixel_scale to define the wcs of the coadd image.

Some possible changes are listed as follows:

  • use coadd_length in units of arcmin to define the Layout object and remove coadd_dims from the input.
  • change the make_coadd_dm_wcs(layout, coadd_pixel_scale, coadd_dims) to output a set of wcs for coadd pathches.
  • enable users to input wcs of single CCD.

@esheldon
Copy link
Collaborator

Sorry, I was just discussing these as future possibilities, not for this PR

@mr-superonion mr-superonion merged commit cd57e25 into LSSTDESC:master Dec 18, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants