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 dimension to gain mask if ndim equals 1 #72

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kbruegge
Copy link
Member

Fixes an error which occurs when the gain_selector.select_gains returns an unexpected dimension.
Lets call this a hotfix since I have no idea what this is doing exactly. And why.

@vuillaut
Copy link
Member

Hi @mackaiver
I believe the expected dimension is 1.
The gain_mask is not really a mask anymore but an array telling you what channel to use for each pixel.

@vuillaut
Copy link
Member

Hi @mackaiver
I believe the expected dimension is 1.
The gain_mask is not really a mask anymore but an array telling you what channel to use for each pixel.

Sorry I was mixing ctapipe master version and v0.6.2.
If the gain selection is behaving in an unexpected way, I'd rather have the code breaking.

@kbruegge
Copy link
Member Author

I did this on ctapipe v0.6.2.
For some telescopes it worked, for others it didn't. I did not inverstigate further.

The dl1-data-handler does not work on the current ctapipe version I as far as I know. Ever since the EventSources have been refactored in ctapipe I guess.

@vuillaut
Copy link
Member

I did this on ctapipe v0.6.2.
For some telescopes it worked, for others it didn't. I did not inverstigate further.

The dl1-data-handler does not work on the current ctapipe version I as far as I know. Ever since the EventSources have been refactored in ctapipe I guess.

Yes many changes in ctapipe are not compatible anymore (e.g. gain selection that's why I was confused at first).

Could you rather open an issue and tell me (if you know) for which cameras it fails please?

@kbruegge
Copy link
Member Author

Ok. Good idea. Unfortunately I'm rather busy at the moment. Maybe @riwim has some time to spare?

@riwim
Copy link

riwim commented Jul 10, 2019

I will definitely try. As of now, I have just been a user of the handler. It will take some time for me to become acquainted with the project and be able to draft a reasonable issue for the very case.

Edit: I'm sorry but I can't find out how to test for this problem.

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.

3 participants