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

Repeated solve #3

Closed
wants to merge 2 commits into from
Closed

Repeated solve #3

wants to merge 2 commits into from

Conversation

Nathaniel-github
Copy link
Collaborator

No description provided.

Copy link

codecov bot commented Dec 5, 2024

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@Nathaniel-github
Copy link
Collaborator Author

@aarmey I was talking to Andrew today about adding in a repeated solve for the projection matrix in case it was simply a convergence to the global minimum issue, so this code is adding in that change. From my initial runs it seems like the method is no longer failing (I capped it at 5 max repeated solves for now) which leads me to believe our objective function is not convex. We had a brief discussion about this during our last subgroup but if that is not an issue then this should be the finishing touch on the projection method.

@Nathaniel-github Nathaniel-github marked this pull request as draft December 5, 2024 21:44
@Nathaniel-github Nathaniel-github marked this pull request as ready for review December 5, 2024 21:45
@aarmey
Copy link
Member

aarmey commented Dec 6, 2024

@Nathaniel-github I would prefer to not worry about it giving a local minimum for now. Keep in mind that it will give an improved result after fitting. Also, this fitting step will be run many times within Pf2. We can revisit convergence once you have a full method.

@andrewram4287
Copy link
Collaborator

@Nathaniel-github Let's keep this mind, but I will close the PR for now.

@andrewram4287 andrewram4287 deleted the repeat_solve branch December 6, 2024 19:05
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