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

Damp initial iterations of Broyden's method used in ID parameter control #6378

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

iago-mendes
Copy link
Member

Proposed changes

This PR modifies the initial data parameter control so that it doesn't go off-bounds in the q3 run of arXiv:1506.01689. We tried different modifications to Broyden's method, summarized in the plot below. The one that seems to work best for now is to damp the initial iterations of the Jacobian updates. We plan to make this damping more robust in the future.

root_finding_methods

Upgrade instructions

Code review checklist

  • The code is documented and the documentation renders correctly. Run
    make doc to generate the documentation locally into BUILD_DIR/docs/html.
    Then open index.html.
  • The code follows the stylistic and code quality guidelines listed in the
    code review guide.
  • The PR lists upgrade instructions and is labeled bugfix or
    new feature if appropriate.

Further comments

@knelli2 knelli2 added the cli/pybindings Command line interface & Python bindings label Nov 15, 2024
@knelli2 knelli2 added this to the BBH Pipeline Automation milestone Nov 15, 2024
@nilsdeppe
Copy link
Member

Sorry, it's not clear which of the ones you mean. There are 4 curves that damp using the Jacobian. It also seems like delaying the P & C updates 2 iterations is even better, so I'm surprised by your recommendation. Naively I'd expect combining P&C delay, and then after 3 iterations damping with 1-e^{-k} to prove the best combination of the ideas presented.

@iago-mendes
Copy link
Member Author

Apologies for the non-clarity. Out of the 4 curves that damp the Jacobian, this PR uses the one with -k/2 in the exponent (pink curve).

Indeed, the initial delay of Padm and CoM seems to perform better in this case. My understanding from conversations I've had with @nilsvu is that it would be more straightforward to modify the damping approach in the future to make the control scheme more robust, instead of changing it to work for each different configuration.

I am curious about how @nilsdeppe's suggestion performs. I'll experiment with it and share updates here as soon as possible. I'm also happy to explore other variations.

@teukolsky
Copy link
Contributor

@iago-mendes I've had good success in other problems using Broyden's method by initializing the Jacobian using numerical finite differences instead of using the identity matrix. The idea is that using the identity matrix, the first N iterations are building up an approximation to the Jacobian (N = # of parameters to be found), so you need at least N iterations before you start converging. Instead, using finite differencing also takes N function evaluations to get the Jacobian, and it should be more accurate that the first N Broyden steps. I'm not sure this is worth exploring in your case, since the number of iterations is reasonably small, but I wanted to mention it for completeness.

@iago-mendes
Copy link
Member Author

Thanks for the suggestion, @teukolsky! Here, we've carefully chosen our free data so that the Jacobian is approximately an identity matrix. In fact, in the Newtonian approximation, it is exactly an identity matrix. If interested, I explain how it works on this report (page 7). Hence, I believe it's not a problem to initialize the Jacobian in this way.

I've experimented with @nilsdeppe's suggestion, but 1-exp(-k) approaches 1 soon after the first couple of iterations, so the damping isn't doing much. While doing these tests again, I realized that what I had previously called "delay first 2 iterations" was actually iterations < 2, so it should be called "delay first iteration".

After talking to @nilsvu about the different options, we agree that none of them are as robust as we eventually want them to be, so it makes sense to go with the one that converges the fastest for now. Hence, I've updated this PR to the delay approach.

Our current focus is on controlling Eadm and Jadm, which will be relevant for hyperbolic encounters. Once that's done, we want to do a more complete study in parameter space. At that time, we'll want to find a more robust approach for this.

@teukolsky
Copy link
Contributor

Ah! The fact that your initial Jacobian is a good approximation explains why you're in the convergent regime immediately. I didn't understand why there was good convergence before N iterations.

@nilsdeppe
Copy link
Member

Sounds great! Thanks for the very thorough update and looking into that! I think the plan sounds good!

@nilsvu since this is python code and in things you wrote, would you mind reviewing the code itself?

@nilsvu nilsvu merged commit eeafc0d into sxs-collaboration:develop Nov 22, 2024
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli/pybindings Command line interface & Python bindings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants