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

CameraReadout.from_file() should fail if no camerareadout exists #1450

Closed
kosack opened this issue Oct 9, 2020 · 2 comments · Fixed by #1703
Closed

CameraReadout.from_file() should fail if no camerareadout exists #1450

kosack opened this issue Oct 9, 2020 · 2 comments · Fixed by #1703
Milestone

Comments

@kosack
Copy link
Contributor

kosack commented Oct 9, 2020

This caused a lot of confusion, and means most of the tests are not correct (since they fail to read a correct CameraReadout and fall back to a generic one that doesn't look at all like a Cherenkov pulse):

From readout.py:

        except FileNotFoundError:
            # TODO: remove case when files have been generated
            logger.warning(
                f"Resorting to default CameraReadout,"
                f" File does not exist: ({tabname})"
            )
            reference_pulse_shape = np.array([norm.pdf(np.arange(96), 48, 6)])
            return cls(
                camera_name=camera_name,
                sampling_rate=u.Quantity(1, u.GHz),
                reference_pulse_shape=reference_pulse_shape,
                reference_pulse_sample_width=u.Quantity(1, u.ns),
            )

So if you create a camera like:

camera = CameraDescription.from_name("LST")

You get a crazy reference pulse shape that doesn't reflect reality (it's 10x too wide and not properly normalized), and this is used in tests internally. I think the correct behavior would be to fail with an error, and we should add correct *.camerareadout.fits.gz files in ctapipe-extra so that doesn't happen.

image

@kosack
Copy link
Contributor Author

kosack commented Oct 9, 2020

Of course, we don't have camera_readout defs for some of the test cameras (HESS, Whipple-X, etc). So perhaps instead of simply failing, it should just not create the CameraReadout (leave it as None), that way tests that do not need timing will still work, but those that do not should fail on cameras with no timing info

@kosack
Copy link
Contributor Author

kosack commented Oct 9, 2020

As a quick fix, we should just add *.camreadout.fits.gz files to ctapipe-extra. I opened #1451 to facilitate that.

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 a pull request may close this issue.

2 participants