search

roman-corgi / corgidrp

Data Reduction Pipeline for the Roman Coronagraph Instrument
BSD 3-Clause "New" or "Revised" License
5 stars 4 forks source link

Boresight calibration #113

Closed semaphoreP closed 3 months ago

semaphoreP commented 5 months ago

Making this for @manduhmia

hsergi commented 4 months ago

@manduhmia @semaphoreP: I've read thru the code and unit test. Looks good to me. Just wondering about the 72 arcsec radius to search for the target star. match_sources() has this hard coded parameter rad=0.02 # deg. Is it a good idea to have it hardcoded? How does it compare with the CBE of Roman’s observatory pointing (RA, DEC)? P.S. Same (hard coded) radius value is used in mocks.create_astrom_data()

manduhmia commented 3 months ago

@manduhmia @semaphoreP: I've read thru the code and unit test. Looks good to me. Just wondering about the 72 arcsec radius to search for the target star. match_sources() has this hard coded parameter rad=0.02 # deg. Is it a good idea to have it hardcoded? How does it compare with the CBE of Roman’s observatory pointing (RA, DEC)? P.S. Same (hard coded) radius value is used in mocks.create_astrom_data()

That radius value is kind of arbitrary, I just needed to set a large enough search radius to not 'miss' any stars in the source matching function-- I made it tunable so that the user can now decide how big that should be. I also made it tunable in the mocks.create_astrom_data(), although that one is just used to speed up computation in translating LMC sources to pixel space in the mock image.

semaphoreP commented 3 months ago

@hsergi if you think the modifications look good to you, can you give your approval to the PR? If not, let us know if there's anything else.