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

refactor: wrap fit result in class #254

Merged
merged 3 commits into from
Mar 29, 2021
Merged

refactor: wrap fit result in class #254

merged 3 commits into from
Mar 29, 2021

Conversation

redeboer
Copy link
Member

@redeboer redeboer commented Mar 26, 2021

Wrapped the optimize dict output in a FitResult class. It's a rather simple attr data container that wraps what was previously in a dict form. Optimizer-specific info has been put under a specifics attribute.

@redeboer redeboer added the ⚠️ Interface Breaking changes to the API label Mar 26, 2021
@redeboer redeboer added this to the Release 0.2.2 milestone Mar 26, 2021
@redeboer redeboer requested a review from spflueger March 26, 2021 17:57
@redeboer redeboer self-assigned this Mar 26, 2021
@codecov
Copy link

codecov bot commented Mar 26, 2021

Codecov Report

Merging #254 (5303b84) into master (3c788a0) will decrease coverage by 1.23%.
The diff coverage is 61.53%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #254      +/-   ##
==========================================
- Coverage   81.17%   79.94%   -1.24%     
==========================================
  Files          13       13              
  Lines         712      743      +31     
  Branches      119      124       +5     
==========================================
+ Hits          578      594      +16     
- Misses         96      111      +15     
  Partials       38       38              
Flag Coverage Δ
unittests 79.94% <61.53%> (-1.24%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/tensorwaves/interfaces.py 82.14% <55.88%> (-17.86%) ⬇️
src/tensorwaves/optimizer/minuit.py 93.22% <100.00%> (ø)
src/tensorwaves/optimizer/scipy.py 93.10% <100.00%> (ø)

@redeboer
Copy link
Member Author

One thing that's still missing is the covariance matrix. We have to think about a data type for that though.

@redeboer
Copy link
Member Author

So the way I see this PR now is that it only (1) replaces the dict fit result with a class and (2) offers missing additional info (particularly the covariance matrix or hessian). This means that #165 is not closed.

Btw, the fact that the dict has been replaced with a class, helped fix one typo:

"interations": res.nit,

@redeboer redeboer changed the title refactor: implement FitResult class refactor: wrap fit result in class Mar 29, 2021
@redeboer redeboer merged commit fa7fb37 into master Mar 29, 2021
@redeboer redeboer deleted the FitResult branch March 29, 2021 09:53
@redeboer redeboer modified the milestones: 0.2.4, 0.2.2 May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ Interface Breaking changes to the API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants