Skip to content

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
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


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

"""
pars = self._cached_pars

@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

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


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
@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