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

Lineprofiles: better handling of initial guess in root finder #72

Closed
phajy opened this issue Feb 6, 2023 · 2 comments
Closed

Lineprofiles: better handling of initial guess in root finder #72

phajy opened this issue Feb 6, 2023 · 2 comments
Assignees
Labels
analysis Issue or pull request related to an analysis technique or method enhancement New feature or request

Comments

@phajy
Copy link
Contributor

phajy commented Feb 6, 2023

I was going to post this as a follow-up in Precision tolerances but have decided to make it a "new" issue instead.

I've encountered a (possibly?) precision-related error: Roots.ConvergenceFailed("Algorithm failed to converge"). Here's an example to investigate (based on this example).

a = 0.0
d = GeometricThinDisc(0.0, 400.0, π / 2)
u = @SVector [0.0, 1000.0, deg2rad(60), 0.0]
m = KerrMetric(1.0, a)

# maximal integration radius
maxrₑ = 200.0

# emissivity function
ε(r) = r^(-3)

# g grid to do flux integration over
gs = range(0.0, 1.2, 500)
_, flux = lineprofile(gs, ε, m, u, d, maxrₑ = maxrₑ)

I've briefly experimented with some of the hidden parameters, e.g., increasing the number of g* points or reducing the tolerance but with no success. This differs from the example by having a much larger outer radius where $g_\text{max} - g_\text{min}$ will be very small and close to zero.

@phajy phajy added bug Something isn't working integrator Issue or pull request related to integration methods labels Feb 6, 2023
@fjebaker
Copy link
Member

fjebaker commented Feb 7, 2023

I am able to reproduce. These sorts of precision errors in the root finder normally mean the extrema of the bracketing interval which sets up the precision tracer missed the disc. This often happens when roughly $r_\text{e} \geq \frac{1}{2} r_\text{d}$.

A simple fix is to set, e.g.

d = GeometricThinDisc(0.0, 800.0, π / 2)

But I'll leave this issue open with the addition that

  • This should either generate a more friendly error message; or
  • Use a better heuristic when calculating the bracketing intervals for continuous discs / find appropriate behaviour when the disc is sparse.

For the latter, the offending line is

which could probably be safely set to d.outer_radius + Δ, where Δ is just a safety offset. In the past this had to scale to help the root finder converge, but we're using a better default algorithm now, so we might be able to get away with just using the same maximum offset for each emission radius.

@fjebaker fjebaker added enhancement New feature or request analysis Issue or pull request related to an analysis technique or method and removed bug Something isn't working integrator Issue or pull request related to integration methods labels Feb 7, 2023
@fjebaker fjebaker changed the title Convergence problem Lineprofiles: better handling of initial guess in root finder Feb 7, 2023
@fjebaker
Copy link
Member

Fixed in #107

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analysis Issue or pull request related to an analysis technique or method enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants