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

feat: least-squares iterator #138

Merged
merged 1 commit into from
Feb 27, 2025

Conversation

maxdinkel
Copy link
Member

Description and Context

This PR replaces the old LMIterator implementation with a more general LeastSquaresIterator based on the SciPy implementation. This allows one to choose between three algorithms (lm, dogbox, trf) for optimizing a least squares objective.

Related Issues and Pull Requests

  • Closes
  • Blocks
  • Is blocked by
  • Follows
  • Precedes
  • Related to
  • Part of
  • Composed of

How Has This Been Tested?

I removed the unit tests for the old LMIterator, but adapted the existing integration tests to test all algorithms (lm, dogbox, trf).

Checklist

  • My commit messages mention the appropriate issue numbers (advised but optional).
  • I mention the appropriate issue numbers in this PR.
  • I updated documentation where necessary.
  • I have added tests to cover my changes.

Additional Information

Interested Parties

@sbrandstaeter

Possible reviewers:

Other interested parties:

@maxdinkel maxdinkel requested a review from gilrrei February 25, 2025 08:29
@maxdinkel maxdinkel self-assigned this Feb 25, 2025
@maxdinkel maxdinkel force-pushed the refactor-lm-iterator branch from 49a51a8 to 7bbfda4 Compare February 25, 2025 16:40
gilrrei
gilrrei previously approved these changes Feb 26, 2025
Copy link
Member

@gilrrei gilrrei left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@leahaeusel leahaeusel 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, thanks for updating this iterator! I just have two minor comments.

@maxdinkel maxdinkel merged commit 6cccfde into queens-py:main Feb 27, 2025
3 checks 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