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-43674: Switch comCamSim to use the LSSTCam distortion model #522

Merged
merged 1 commit into from Apr 4, 2024

Conversation

isullivan
Copy link
Contributor

No description provided.

@jmeyers314
Copy link
Contributor

What's the long term plan here? I anticipate we'll make more ComCamSim sims going forward, presumably without accidentally using the wrong optics. For those we'll want the current numbers, not the ones in this PR.

(I'm fine with this as a temporary override, of course).

Copy link
Member

@TallJimbo TallJimbo left a comment

Choose a reason for hiding this comment

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

Looks good!

To partially answer @jmeyers314 's question, I think this is a reminder that we need to eliminate code that relies on obs_lsst to get the camera object in favor of code that reads the butler camera dataset. And then we can give the butler camera dataset real bounded validity ranges (corresponding to old and new sims) and see what falls apart. Something surely will, so that won't be pleasant, but we need to get this sorted before we experience a real change in a real camera.

@isullivan isullivan merged commit 5c5439f into main Apr 4, 2024
3 checks passed
@isullivan isullivan deleted the tickets/DM-43674 branch April 4, 2024 01:14
@timj
Copy link
Member

timj commented Apr 4, 2024

It seems odd to say that ComCamSim is using LSSTCam model and not LSSTComCam model.

@TallJimbo
Copy link
Member

Oh, it's definitely odd. It's just the same kind of odd as the simulations; that's where the mistake occurred.

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.

None yet

4 participants