-
Notifications
You must be signed in to change notification settings - Fork 7
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
Added the Radford Impurity Density model to the SSD extension #72
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #72 +/- ##
==========================================
+ Coverage 33.96% 34.40% +0.43%
==========================================
Files 28 26 -2
Lines 1946 1959 +13
==========================================
+ Hits 661 674 +13
Misses 1285 1285 ☔ View full report in Codecov by Sentry. |
Project.toml
Outdated
@@ -11,6 +11,7 @@ IntervalSets = "8197267c-284f-5f27-9208-e0e47529a953" | |||
JSON = "682c06a0-de6a-54ab-a142-c8b1cf79cde6" | |||
LRUCache = "8ac3fa9e-de4c-5943-b1dc-09c6b5f20637" | |||
LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" | |||
LsqFit = "2fda8390-95c7-5789-9bda-21331edee243" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oschulz would you be ok with adding LsqFit as dependency until the fitted impurity density values make it into the metadata?
) | ||
else | ||
@warn "No crystal metadata found for detector $(meta.name)" | ||
# TODO: Implement a default impurity density for cases without crystal metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would still need to be tested and done, I can take care of this
We also might wanna add a keyword argument such that the user can choose whether they want to use the RadfordImpurityDensity from the crystal metadata or not |
I closed this by accident when trying to resolve the merge conflicts. Find the follow-up PR here: #74 |
Until now, each detector was characterised by a constant impurity density. By using the Radford Impurity Density model, we can get a more realistic parametrisation of the detectors.
The
impurity_density
field now contains:name
: "radford"params
: The five resulting parameters from the fit, as well as the detector offsetThis implementation introduces a new package dependency to LegendDataManagement, namely LsqFit.