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

event_count repeated in Phyllis result #705

Closed
jacg opened this issue Mar 10, 2020 · 4 comments
Closed

event_count repeated in Phyllis result #705

jacg opened this issue Mar 10, 2020 · 4 comments

Comments

@jacg
Copy link
Collaborator

jacg commented Mar 10, 2020

Is Phyllis being used?

Its result contains 4 values, two of which are the same value repeated under different names: events_in and event_count are both the result of event_count.future:

        result = dict(events_in   = event_count     .future,
                      spe         = accumulate_light.future,
                      dark        = accumulate_dark .future,
                      event_count =      event_count.future)

(Additionally, the second event_count is misaligned.)

@jacg
Copy link
Collaborator Author

jacg commented Mar 10, 2020

... and exactly the same problem appears it Trude.

In fact, the amount of code duplication between Phyllis and Trude looks quite scary! (It's not immediately obvious what the best way of avoiding this would be, but we can definitely do better.)

@carmenromo
Copy link
Collaborator

Yes, both of them are being used! They compute the spectra for SiPMs (Trude) and PMTs (Phyllis)

@jacg
Copy link
Collaborator Author

jacg commented Mar 10, 2020

Oh dear. Is the event_count being used?

jwaiton pushed a commit that referenced this issue Jun 3, 2024
#873

[author: paolafer]

This PR removes an unnecessary variable from two cities, which was
repeated. Closes issue #705.

[reviewer: gonzaponte]

Good!
@gonzaponte
Copy link
Collaborator

Done in #873 and #880.

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

No branches or pull requests

3 participants