Skip to content
This repository has been archived by the owner on Jun 5, 2024. It is now read-only.

Fix s2 photon timing bug #434

Merged
merged 3 commits into from
Apr 18, 2024
Merged

Fix s2 photon timing bug #434

merged 3 commits into from
Apr 18, 2024

Conversation

FaroutYLq
Copy link
Contributor

What is the problem / what does the code in this PR do
Inspired by solution in fuse here, trying to solve the s2 photon timing bug. The detailed mechanism why it fixed things is unclear but we don't care much for a legacy package. Below is before and after the fix:

image
image

WFSim needs you:

  • Please add a test for this PR, as a bare minimum, make sure it's covered in coveralls!
  • Can you add a docsting to all your functions?

Pay attention:

  • Due to databases being needed for testing, making a PR from your own fork will typically NOT run the tests. If you then merge master might break

@FaroutYLq FaroutYLq requested a review from ramirezdiego March 20, 2024 16:38
@FaroutYLq FaroutYLq added the bug Something isn't working label Mar 20, 2024
@coveralls
Copy link

Coverage Status

coverage: 82.78% (+0.1%) from 82.67%
when pulling bb835b3 on fix_s2_photon_timing_bug
into 2317065 on master.

@ramirezdiego
Copy link
Collaborator

Funny:

FAILED tests/test_wfsim.py::test_sim_1T - ValueError: Found n_hits less than tight_coincidence
============ 1 failed, 20 passed, 439 warnings in 779.44s (0:12:59) ============
Error: Process completed with exit code 1.

@FaroutYLq
Copy link
Contributor Author

OK so it somehow passed after running again. I would recommend just merging it then.

@FaroutYLq FaroutYLq merged commit 8180bda into master Apr 18, 2024
6 checks passed
@ramirezdiego ramirezdiego deleted the fix_s2_photon_timing_bug branch April 22, 2024 08:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants