Skip to content

Add optional x1 to provide the scipy secant method with a second guess#3515

Closed
lewisgross1296 wants to merge 1 commit intoopenmc-dev:developfrom
lewisgross1296:allow_second_guess_to_secant_solve
Closed

Add optional x1 to provide the scipy secant method with a second guess#3515
lewisgross1296 wants to merge 1 commit intoopenmc-dev:developfrom
lewisgross1296:allow_second_guess_to_secant_solve

Conversation

@lewisgross1296
Copy link
Contributor

Description

Closes #3512. I tried to find existing tests that involve search_for_keff but I'm not sure we have any regression tests for this. If I were to implement one specifically for this, I'd just query the guesses returned and make sure that guess[0] and guess[1] are what the user supplies. I guess this opens up a question to how we'd like to test search_for_keff (if we'd like to)

Locally, this modification seems to work, but if people would like a demo, I can provide a zip of an example demonstrating the capability, which could maybe be inspiration for a test.

Checklist

  • I have performed a self-review of my own code
  • [ ] I have run clang-format (version 15) on any C++ source files (if applicable)
  • [x ] I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

Copy link
Contributor

@paulromano paulromano left a comment

Choose a reason for hiding this comment

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

Thanks for proposing this @lewisgross1296. Note that there is some recent discussion on search_for_keff in #2693 and #1656. I think the direction I'd like to go is to 1) use scipy.optimize.root_scalar and allow the user to pass through whatever arguments are needed for a given method (which would allow passing x1 as you've proposed here), and 2) also add a method for uncertainty-aware root-finding (which would likely become the default).

@lewisgross1296
Copy link
Contributor Author

Thanks for directing me towards that PR! I'm very interested how that turns out, as we're planning on doing some multiphysics search for keff work soon. Do you propose for the changes to allow the user to specify args to happen there instead of this PR? It looks like root_scalar takes both args() as well as an x1 argument

@paulromano
Copy link
Contributor

Yes, I think those changes will probably get incorporated in a separate PR, so I'll go ahead and close this one for now.

@paulromano paulromano closed this Aug 1, 2025
@lewisgross1296 lewisgross1296 deleted the allow_second_guess_to_secant_solve branch August 1, 2025 14:51
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.

Add capability for second guess (x1) to search_for_keff secant solve

2 participants