search

XENONnT / fuse

XENON Framework for Unified Simulations of Events
BSD 3-Clause "New" or "Revised" License
6 stars 1 forks source link

Update BetaYields #192

Closed HenningSE closed 4 months ago

HenningSE commented 6 months ago

This PR is a copy of the work done in https://github.com/XENONnT/fuse/pull/168. A new PR was opened to allow the automatic tests to run.

HenningSE commented 6 months ago

Description of the PR from @cfuselli: https://github.com/XENONnT/fuse/pull/168#issue-2194743813

coveralls commented 6 months ago

Pull Request Test Coverage Report for Build 8701253557

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
fuse/plugins/micro_physics/yields.py 11 27 40.74%
<!-- Total: 11 27 40.74% -->
Files with Coverage Reduction New Missed Lines %
fuse/plugins/micro_physics/yields.py 1 38.11%
<!-- Total: 1 -->
Totals Coverage Status
Change from base Build 8701161694: 0.5%
Covered Lines: 2089
Relevant Lines: 2903
💛 - Coveralls
coveralls commented 6 months ago

Pull Request Test Coverage Report for Build 9694781680

Details

Totals Coverage Status
Change from base Build 9694329996: 0.006%
Covered Lines: 2603
Relevant Lines: 3232
💛 - Coveralls
HenningSE commented 6 months ago

Thanks for the nice update of the BetaYields plugin @cfuselli. There are a few changes I would like to make before we merge the PR:

HenningSE commented 6 months ago

The last commit added a test for the BetaYields plugin. The test will fail as the config arguments are not properly set yet.

cfuselli commented 5 months ago

Hey @HenningSE , @ramirezdiego, @michaweiss89 . On this PR I would like to propose one change: remove the g1 and g2 dependency by including it directly in the file with the spline function. I think this is better because the most important thing is that the g1 and g2 match the ones used to make the function. Do you agree? Does it make sense?

cfuselli commented 4 months ago

@HenningSE I added a dummy function in the tests so the automatic tests finally pass. I also removed dependency from g1 and g2. The file with the function for the yields is still to be passed manually with a file path, because I think it still needs some work before uploading it. I believe the PR can go like this for the moment, to make it easier to work and test the functionality?

HenningSE commented 4 months ago

Hi @cfuselli, the dummy function is a good idea for the tests, thanks! I think the PR is good to go and we can merge together with all the other high energy-focused PRs very soon.

cfuselli commented 4 months ago

I tested it and it seems like it runs smoothly:)