Skip to content

Conversation

@SouthEndMusic
Copy link
Collaborator

@SouthEndMusic SouthEndMusic commented Aug 26, 2024

BackTracking as relaxation is now enabled again, with a thin wrapper to reject it when the residual gets worse. Upstream issue:

SciML/OrdinaryDiffEq.jl#2442

@visr
Copy link
Member

visr commented Aug 27, 2024

Since the sensible gradients don't fix the Crystal model with BackTracking, I guess we should not enable that yet?

How do these gradients compare with Shampine's advice in #1705 (comment), should we bring that back as well in this PR?

@SouthEndMusic
Copy link
Collaborator Author

I added Shampine's advice, but it doesn't seem to change anything

get_du,
AbstractNLSolver,
relax!,
_compute_rhs!,
Copy link
Member

Choose a reason for hiding this comment

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

I guess we may be able to stop using this internal function when we upstream this?

@SouthEndMusic SouthEndMusic changed the title Have more sensible gradients with negative storages Re-enable BackTracking Aug 29, 2024
@SouthEndMusic SouthEndMusic requested a review from visr August 29, 2024 13:03
Copy link
Member

@visr visr left a comment

Choose a reason for hiding this comment

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

Very nice find!

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