search

ICB-DCM / pyPESTO

python Parameter EStimation TOolbox
https://pypesto.readthedocs.io
BSD 3-Clause "New" or "Revised" License
216 stars 47 forks source link

Fix CMAES installation hint #1311

Closed dilpath closed 6 months ago

dilpath commented 6 months ago

Currently instructs users to install cma with pip install pypesto[cma], but this isn't configured in "setup.py".

Now refactored so everything is in terms of cma and CmaOptimizer.

codecov-commenter commented 6 months ago

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 81.88%. Comparing base (d32563b) to head (bc338e0).

Files Patch % Lines
pypesto/optimize/optimizer.py 60.00% 2 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #1311 +/- ## =========================================== - Coverage 84.40% 81.88% -2.52% =========================================== Files 153 153 Lines 12478 12482 +4 =========================================== - Hits 10532 10221 -311 - Misses 1946 2261 +315 ```

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

dilpath commented 6 months ago

Perhaps would be nicer to rename to cma in setup?

Agreed, as well as renaming CmaesOptimizer to CmaOptimizer, since it looks like the other optimizers are named after the package rather than the method. What do you think? Would be an unnecessary breaking change then.

PaulJonasJost commented 6 months ago

Perhaps would be nicer to rename to cma in setup?

Agreed, as well as renaming CmaesOptimizer to CmaOptimizer, since it looks like the other optimizers are named after the package rather than the method. What do you think? Would be an unnecessary breaking change then.

I am fine with that, we can have a period where we still allow the usage of CmaesOptimizer but then with a deprecation warning? Would not be breaking per se but we can then remove it later?