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

Refactor gMAM #25

Merged
merged 6 commits into from
Mar 19, 2024
Merged

Refactor gMAM #25

merged 6 commits into from
Mar 19, 2024

Conversation

oameye
Copy link
Member

@oameye oameye commented Feb 23, 2024

No description provided.

@oameye oameye marked this pull request as draft February 23, 2024 10:09
@oameye
Copy link
Member Author

oameye commented Feb 23, 2024

@reykboerner What is the reason that we have to re-interpolate the path after every optimization path? I guess it ensures continuity of the path? It would be nice if we could enforce this within the Optimizer of Optim.jl. Not sure if it possible tho.

@oameye oameye marked this pull request as ready for review February 23, 2024 12:37
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please explain what this does?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ha yes sorry. Maybe I shouldn't have committed this. This is a configuration file that format the code to the SciML style guidelines. It works with JuliaFormatter.jl. Feel free to delete this. We could opt to add a CI who automatically formats the code. But I know that Juliadynamics is also still arguing what style they want to use.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, I'll omit it now and we can discuss the CI in our next meeting.

@reykboerner
Copy link
Collaborator

Hi @oameye, are there any changes here that change what geometric_min_action_method does or are they all just cosmetic in terms of how the code is written? It's a bit difficult to tell from the PR title and by scrolling through the changes.

@oameye
Copy link
Member Author

oameye commented Mar 18, 2024

All the changes should be cosmetic. I just refactored and reformatted the code. The main changes are:

  • Refactored the interpolation step into its own function
  • Make the two methods dependent on each other, that way we don't have redundant code.
  • Ran a JuliaFormatter.jl with the SciML style.

@reykboerner
Copy link
Collaborator

Great, thanks for the additional description! Merging this now.

@reykboerner reykboerner merged commit 9cb3569 into JuliaDynamics:main Mar 19, 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.

2 participants