legend-exp / legend-pygeom-hpges

Germanium detector geometries for radiation transport simulations
https://legend-pygeom-hpges.readthedocs.io
GNU General Public License v3.0
0 stars 4 forks source link

implement distance to surface and add the surface types to HPGe object #51

Closed tdixon97 closed 2 weeks ago

tdixon97 commented 4 weeks ago

This implements a calculation of the distance between points and the surface.

We should also add.

codecov[bot] commented 4 weeks ago

Codecov Report

Attention: Patch coverage is 68.51852% with 85 lines in your changes missing coverage. Please review.

Project coverage is 77.86%. Comparing base (a7c9b2c) to head (ad7b135). Report is 64 commits behind head on main.

Files with missing lines Patch % Lines
src/legendhpges/utils.py 53.22% 29 Missing :warning:
src/legendhpges/draw.py 0.00% 22 Missing :warning:
src/legendhpges/base.py 91.80% 5 Missing :warning:
src/legendhpges/p00664b.py 54.54% 5 Missing :warning:
src/legendhpges/ppc.py 54.54% 5 Missing :warning:
src/legendhpges/invcoax.py 73.33% 4 Missing :warning:
src/legendhpges/v02160a.py 73.33% 4 Missing :warning:
src/legendhpges/v07646a.py 75.00% 4 Missing :warning:
src/legendhpges/semicoax.py 72.72% 3 Missing :warning:
src/legendhpges/bege.py 83.33% 2 Missing :warning:
... and 1 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #51 +/- ## ========================================== - Coverage 79.74% 77.86% -1.88% ========================================== Files 14 16 +2 Lines 548 768 +220 ========================================== + Hits 437 598 +161 - Misses 111 170 +59 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ManuelHu commented 3 weeks ago

just an idea: maybe it would make sense to split utils into two parts; one that creates geometry (i.e. helpers for contacts, maybe spherical segments later on) and one for consuming geometry (i.e. calculating distances)?

tdixon97 commented 3 weeks ago

Yes probably you are right

gipert commented 3 weeks ago

Another thing to take care of: I think that because of finite numerical precision some hits could be found outside the detector by a tiny amount -> we should push them back in

tdixon97 commented 3 weeks ago

Another thing to take care of: I think that because of finite numerical precision some hits could be found outside the detector by a tiny amount -> we should push them back in

This is not necessarily so trivial, need to compute if a point is inside vs outside, i.e. attach a sign to the distance.

tdixon97 commented 3 weeks ago

I left some comments.

All of this can be very easily sped up with Numba, let's do that.

Now done.

tdixon97 commented 2 weeks ago

Any more issues? Can we merge this ?

ManuelHu commented 2 weeks ago

for me it looks fine, we can always iterate later if we need to.

tdixon97 commented 2 weeks ago

Should have now fixed your last comments @gipert

gipert commented 2 weeks ago

I have removed the pre-commit commits and pushed some cosmetic stuff, getting ready to merge