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

Add support for using toymodel in telescope frame #2349

Merged
merged 5 commits into from
Jun 20, 2023
Merged

Conversation

maxnoe
Copy link
Member

@maxnoe maxnoe commented Jun 13, 2023

missing tests

@maxnoe
Copy link
Member Author

maxnoe commented Jun 13, 2023

Just because I had the script already from #2347

hillas_overlay.mp4

ctapipe/image/toymodel.py Outdated Show resolved Hide resolved
ctapipe/image/toymodel.py Outdated Show resolved Hide resolved
@maxnoe maxnoe marked this pull request as ready for review June 15, 2023 10:53
@maxnoe maxnoe requested a review from LukasNickel June 19, 2023 09:27
ctapipe/image/toymodel.py Show resolved Hide resolved
ctapipe/image/toymodel.py Outdated Show resolved Hide resolved
ctapipe/image/toymodel.py Show resolved Hide resolved
@kosack
Copy link
Contributor

kosack commented Jun 19, 2023

Maybe we should just finish removing the CameraFrame versions of all of this... Then we only support TelescopeFrame, and you can transform into CameraFrame to see the result.

@maxnoe maxnoe added this to the v0.19.3 milestone Jun 20, 2023
@Tobychev
Copy link
Contributor

Hi @maxnoe can you explain what bits are actually about the new functionality? Most of this seems to be more style refactoring so I'm not sure what bit is where it is important to look closely...

@maxnoe
Copy link
Member Author

maxnoe commented Jun 20, 2023

Most of this seems to be more style refactoring so I'm not sure what bit is where it is important to look closely...

Actually, none of this is style refactoring, it's all directly related to support the two frames we can have image parameters in.

In the current main, we use u.quantity_input(x=u.m, ..) to make sure units are valid, however, image parameters can be in either CameraFrame which uses length units on the sensor or TelescopeFrame which uses angles on the sky (in degree).

The units checks prevented the toymodel to be used in the TelescopeFrame, now we support both.

@maxnoe maxnoe merged commit 714ab24 into main Jun 20, 2023
@maxnoe maxnoe deleted the toymodel_tel_frame branch June 20, 2023 14:00
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.

Toymodel does not support geometry / model in telescope frame
4 participants