Skip to content

Tests for DFO minimizer #40

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

Merged
merged 9 commits into from
Aug 1, 2024
Merged

Tests for DFO minimizer #40

merged 9 commits into from
Aug 1, 2024

Conversation

andped10
Copy link
Contributor

Added test for DFO minimizer:

  • test_minimizer_dfo.py

Cleaned up minimizer_base.py

  • by removing abstract methods which not are required but only implemented in all sub-classes
  • _prepare_parameters is just moved

Cleaned up minimizer_dfo.py

  • moved methods
  • no intended changes to logic

@andped10 andped10 added the chore PR label label Jul 30, 2024
@@ -59,21 +57,6 @@ def add_fit_constraint(self, constraint: ObjConstraint):
def remove_fit_constraint(self, index: int) -> None:
del self._constraints[index]

@abstractmethod
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@@ -129,26 +112,6 @@ def evaluate(self, x: np.ndarray, minimizer_parameters: dict[str, float] = None,

return self._fit_function(x, **minimizer_parameters, **kwargs)

def _prepare_parameters(self, parameters: dict[str, float]) -> dict[str, float]:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved below

"""
Convert an `EasyScience.Objects.Base.Parameter` object to an engine Parameter object.
"""

@abstractmethod
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed


@abstractmethod
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@@ -37,108 +40,8 @@ def __init__(self, obj, fit_function: Callable, method: Optional[str] = None):
super().__init__(obj=obj, fit_function=fit_function, method=method)
self._p_0 = {}

def make_model(self, pars: Optional[List] = None) -> Callable:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved below

pars = self._cached_pars
item = {}
for p_name, par in pars.items():

Copy link
Contributor Author

Choose a reason for hiding this comment

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

used more meaningful names

@@ -291,10 +292,8 @@ def _gen_fit_results(self, fit_results, weights, **kwargs) -> FitResults:

return results

def available_methods(self) -> List[str]:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved above

return ['leastsq']

def dfols_fit(self, model: Callable, **kwargs):
@staticmethod
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made a static method where pars are now passed

InspectParameter.POSITIONAL_OR_KEYWORD,
annotation=_empty,
default=default_value,
@staticmethod
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made a static method since its functionality only makes sense for the class

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 good, minor stylistic suggestions raised.

@andped10 andped10 merged commit e3d5e96 into develop Aug 1, 2024
26 checks passed
@andped10 andped10 deleted the 39-add-test-for-minimizer-dfo branch August 1, 2024 03:58
rozyczko pushed a commit to rozyczko/EasyScience that referenced this pull request Aug 15, 2024
* first tests and adjustments

* test and changes from public to private methods

* all tests

* fix tests

* PR response

* typing and test

* revert to prevent circular import

* init file corrected

* code cleaning
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.

2 participants