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

Bug Fixes in MLE fits #73

Merged
merged 48 commits into from
Oct 6, 2024
Merged

Bug Fixes in MLE fits #73

merged 48 commits into from
Oct 6, 2024

Conversation

theHenks
Copy link
Collaborator

This PR prevents the MLE fits from crashing. Changes made inlcude:

  • advanced_time_and_memory_control Possibility to set the maximal used memory and time for a single MLE fit by enabling a specific callback after each step of the iterations
  • Usage of the LBFGS algorithm from Optim.jl with the MoreThuente line search algo from the LineSearches.jl package. the NalderZhang seems to perform in general more unstable sometimes doesn't even find a starting point. There are tunable parameters, but the MoreThuente showed in the current tests on Legend data better results
  • Bug fixes + better debugging

@theHenks theHenks added the bug Something isn't working label Sep 25, 2024
@theHenks theHenks requested a review from fhagemann September 25, 2024 13:19
@theHenks theHenks linked an issue Sep 25, 2024 that may be closed by this pull request

# get fit function with background center
fit_function = get_th228_fit_functions(; background_center = background_center)[fit_func]

# create loglikehood function: f_loglike(v) that can be evaluated for any set of v (fit parameter)
f_loglike = let f_fit = fit_function, h = h
v -> hist_loglike(Base.Fix2(f_fit, v), h)
v -> hist_loglike(x -> x in Interval(extrema(h.edges[1])...) ? f_fit(x, v) : 0, h)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then, also f_loglike_array might need an update..

Copy link

codecov bot commented Oct 6, 2024

Codecov Report

Attention: Patch coverage is 24.69512% with 247 lines in your changes missing coverage. Please review.

Project coverage is 15.01%. Comparing base (79f52b7) to head (74bcf77).
Report is 73 commits behind head on dev.

Files with missing lines Patch % Lines
src/peakstats.jl 26.22% 45 Missing ⚠️
src/fit_calibration.jl 0.00% 35 Missing ⚠️
src/singlefit.jl 0.00% 34 Missing ⚠️
src/aoefit_combined.jl 0.00% 28 Missing ⚠️
src/ctc.jl 0.00% 27 Missing ⚠️
ext/LegendSpecFitsRecipesBaseExt.jl 0.00% 26 Missing ⚠️
src/fit_fwhm.jl 0.00% 24 Missing ⚠️
src/aoe_fit_calibration.jl 0.00% 8 Missing ⚠️
src/lqfit.jl 0.00% 8 Missing ⚠️
src/aoefit.jl 0.00% 5 Missing ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##              dev      #73      +/-   ##
==========================================
- Coverage   15.46%   15.01%   -0.45%     
==========================================
  Files          32       33       +1     
  Lines        2489     2670     +181     
==========================================
+ Hits          385      401      +16     
- Misses       2104     2269     +165     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@theHenks
Copy link
Collaborator Author

theHenks commented Oct 6, 2024

All tests are finally running again. Fixed all fits and improved with more BGMLE.
Issue with fit_aoe_compton_combined for uncertaintites remains.

@theHenks theHenks merged commit 64524fe into dev Oct 6, 2024
10 checks passed
@theHenks theHenks deleted the fitting branch October 6, 2024 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gradient-based minimization and gradients
2 participants