Skip to content

Conversation

@andped10
Copy link
Contributor

@andped10 andped10 commented Aug 1, 2024

Tests for Bumps.

Changed order of method in Bumps to have the public methods appear first. No changes to logic

Changed how imports of new_parameter is done in minimizer_lm_fit.py to prevent circular imports when importing Fitter.

@andped10 andped10 added the chore PR label label Aug 1, 2024
def test_available_methods(self, minimizer: Bumps) -> None:
# When Then Expect
assert minimizer.available_methods() == ['amoeba', 'de', 'dream', 'newton', 'scipy.leastsq', 'lm', 'pt']

Copy link
Member

Choose a reason for hiding this comment

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

The pt method in Bumps is experimental and shouldn't really be used. Also, why is scipy here?

Copy link
Contributor Author

@andped10 andped10 Aug 5, 2024

Choose a reason for hiding this comment

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

OK, we will then have to filter the raw output from bumps.

I guess it is called scipy.leastsq because it utilizes the scipy implementation. From the bumps code it appears that LevenbergMarquardtFit is called scipy.leastsq but there is also an internal version that is referred to as lm. The name for this minimizer will be left as is.

Copy link
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

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

Looks OK. Good improvement to the code.

@andped10 andped10 merged commit fd2218c into develop Aug 6, 2024
@andped10 andped10 deleted the 38-add-tests-for-minimizer-bumps branch August 6, 2024 08:23
rozyczko added a commit to rozyczko/EasyScience that referenced this pull request Aug 15, 2024
PR related fixes

ruff, be quiet.

renamed Job->job directory name

38 add tests for minimizer bumps (easyscience#41)

* tests and adjustments

* handling circular dependency

* Updated minimizer names

* Made ruff happy

* pr response

* more consistent usage of enum

---------

Co-authored-by: henrikjacobsenfys <henrik.jacobsen.fys@gmail.com>

removed unnecessary empty line

minimizer consolidation (easyscience#45)

* minimizer consolidation

* pr response
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore PR label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants