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

Levenberg marquardt cleanup #9

Merged
merged 7 commits into from
May 7, 2024

Conversation

minnerbe
Copy link
Contributor

@minnerbe minnerbe commented May 5, 2024

I recently used the LevenbergMarquardtSolver in a project. Despite the experience being very smooth overall, I noticed some typos and inconsistencies in the documentation that made using the class a bit inconvenient. This PR tries to fix most of those; in particular, I:

  • fixed typos and warnings indicated by my IDE;
  • made two new methods computeSquaredError and fit(<7 arguments>) in LevenbergMarquardtSolver in which the order of the arguments is consistent with the order of arguments in the FunctionFitter interface and LevenbergMarquardtSolver's constructor;
  • deprecated the methods with arguments in different order;
  • precomputed the values and gradients of the function to fit to save some (potentially costly) function evaluations.

Let me know if you think that makes sense or if I can do anything else.

@tinevez
Copy link
Contributor

tinevez commented May 6, 2024

Thanks! I am the original author of these files and approved the changes.
I don't want to break imglib2 policy. @tpietzsch can I merge these changes in master?

@minnerbe
Copy link
Contributor Author

minnerbe commented May 6, 2024

Great! I was really glad that this functionality exists within the ImgLib2 universe, so thank you for providing it in the first place 🙏

@tpietzsch tpietzsch merged commit 29e1bcd into imglib:master May 7, 2024
1 check passed
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.

3 participants