Skip to content

Rotate events for more statistics#40

Draft
StFroese wants to merge 28 commits into
mdebony:mainfrom
StFroese:rotate_events
Draft

Rotate events for more statistics#40
StFroese wants to merge 28 commits into
mdebony:mainfrom
StFroese:rotate_events

Conversation

@StFroese

@StFroese StFroese commented Nov 12, 2024

Copy link
Copy Markdown
Contributor

For stereoscopic telescopes like MAGIC, it is necessary to create background models for multiple
azimuth bins since the stereo trigger regions in the FoV changes in AltAz for different pointings.
Alternatively, Simone has shown that is possible to take all events in FoVAltAz and rotate them accordingly (see here chapter 14 &13, https://eldorado.tu-dortmund.de/items/e77bd429-1755-4f37-8733-dc7518376320)

This PR applies this idea to the exclusion method by rotating all events to a specific observations (the Az of the observation) and additionally rotating the exclusion region accordingly.

Some things that are still not done:

  • Show some plots
  • Roate Ellipse exclusion regions, currently only Circle exclusion regions is rotated

Also this doesn't feel like the right place where I implemented it. On the one it I think should add it for the OFF data method too and I think this probably be a complete new method. What's your opinions?

@StFroese

Copy link
Copy Markdown
Contributor Author

also this currently gammapy v1.2

@gabemery

Copy link
Copy Markdown
Collaborator

@StFroese This is an interesting feature, but with all the automatic changes from your IDE (I guess), it is very hard to read. Plus, such coding style changes should be discussed in an independent issue.

@StFroese

StFroese commented Nov 13, 2024

Copy link
Copy Markdown
Contributor Author

@gabemery ah sry, if you tell me the code style I can reformat :)

@gabemery

Copy link
Copy Markdown
Collaborator

We don't have a defined code style yet, but here you change 1200 lines in total for a feature which is probably significantly less.
In addition, you are not starting from an up-to-date branch, and it appears in the diff for some reason (e.g. the diff of init.py is already merged).

So could you restart this PR with only your changes, deactivating the automatic reformatting of full files? And starting from the current main?
This would make it easier to review.

@StFroese
StFroese marked this pull request as draft November 26, 2024 14:26
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