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

feat: add scipy Optimizer adapter #231

Merged
merged 12 commits into from
Mar 17, 2021
Merged

feat: add scipy Optimizer adapter #231

merged 12 commits into from
Mar 17, 2021

Conversation

spflueger
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Mar 16, 2021

Codecov Report

Merging #231 (d41e7fc) into master (96cb06f) will increase coverage by 0.42%.
The diff coverage is 90.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #231      +/-   ##
==========================================
+ Coverage   81.23%   81.65%   +0.42%     
==========================================
  Files          10       12       +2     
  Lines         453      507      +54     
  Branches       61       64       +3     
==========================================
+ Hits          368      414      +46     
- Misses         64       71       +7     
- Partials       21       22       +1     
Flag Coverage Δ
unittests 81.65% <90.00%> (+0.42%) ⬆️

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

Impacted Files Coverage Δ
src/tensorwaves/optimizer/__init__.py 71.42% <66.66%> (-28.58%) ⬇️
src/tensorwaves/optimizer/scipy.py 87.23% <87.23%> (ø)
src/tensorwaves/optimizer/_parameter.py 100.00% <100.00%> (ø)
src/tensorwaves/optimizer/minuit.py 92.72% <100.00%> (-2.21%) ⬇️

@jonas-eschle
Copy link
Contributor

Hey, that looks nice, but I think we should have a chat again in order to avoid duplication (as we're adding an improved interface in zfit right now

@spflueger
Copy link
Member Author

Hey, that looks nice, but I think we should have a chat again in order to avoid duplication (as we're adding an improved interface in zfit right now

Hey Jonas, thanks for the comment. Sure, how about next week? We still have to prepare some things for friday.

Copy link
Member

@redeboer redeboer left a comment

Choose a reason for hiding this comment

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

GJ! Some inline comments, but I approve.
There are similarities between the minuit and scipy submodules now, we can think about how to clean that up in a follow-up PR.

tests/optimizer/test_scipy.py Outdated Show resolved Hide resolved
tests/optimizer/test_parameter_flattener.py Show resolved Hide resolved
src/tensorwaves/optimizer/scipy.py Outdated Show resolved Hide resolved
setup.cfg Show resolved Hide resolved
@spflueger spflueger merged commit 7ad175f into master Mar 17, 2021
@spflueger spflueger deleted the scipy branch March 17, 2021 11:49
@redeboer redeboer added this to the Release 0.2.1 milestone Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants