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

[WIP] Use Runge-Kutta-Fehlberg 7(8) integrator in SphericalDiffuse #94

Closed

Conversation

robertodr
Copy link
Member

This PR aims at replacing the Bulirsch-Stoer adaptive integrator with the non-adaptive Runge-Kutta-Fehlberg 7(8) in the calculation of the radial part of the spherical diffuse Green's function.
See also here

Motivation and Context

The non-adaptive integrator affords faster solution of the radial differential equation, thus allowing the use of more spherical harmonics in the expansion of the radial part.
The issues I noticed in the initial implementation, i.e. that non-adaptive integrators were not robust enough with respect to small widths of the interface, was due to a buggy implementation of the splineInterpolation function. This has now been fixed, based on work we did with @ilfreddy.

Todos

  • Developer Interest
    • Fix possible bug in splineInterpolation.
    • Switch from Bulirsch-Stoer to Runge-Kutta-Fehlberg 7(8).
    • Adjust reference values in the green_spherical_diffuse test

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Questions

  • @ilfreddy check if this makes any difference on run time for your test cases.

Status

  • Ready to go

@robertodr robertodr requested a review from ilfreddy August 18, 2017 17:43
@robertodr robertodr force-pushed the runge_kutta_fehlberg78 branch 2 times, most recently from 52336af to a245bf0 Compare August 25, 2017 07:00
@MinazoBot
Copy link

MinazoBot commented Aug 25, 2017

2 Warnings
⚠️ PR is classed as Work in Progress
⚠️ Consider adding supporting documentation to this change. Documentation sources can be found in the doc directory.

Generated by 🚫 Danger

We also check if we are either end of the sorted array of
input parameters, so that we don't fall off of it when
interpolating.
@robertodr robertodr force-pushed the runge_kutta_fehlberg78 branch from 9cc6e15 to e542b49 Compare August 26, 2017 08:18
Runge-Kutta-Fehlberg 7(8) replaces the Bulirsch-Stoer integrator.
RKF78 is non-adaptive and is cheaper than BS. It is possible to use more
spherical harmonics in the expansion of the Green's function radial
part.
@robertodr robertodr force-pushed the runge_kutta_fehlberg78 branch from e542b49 to 191ef67 Compare August 26, 2017 11:53
@robertodr
Copy link
Member Author

Seems to work fine so far (Travis complains because of some Boost headers) I think I'll bring this one a step further and introduce a simpler implementation of RKF-78 and the Legendre polynomials. We can then expunge Boost.Math.

@bast
Copy link
Member

bast commented Aug 27, 2017

Nice!

@ilfreddy
Copy link
Collaborator

I think this is now obsolete because #109 is a branch which originally started from this one.

@robertodr
Copy link
Member Author

Superseded by #109. Closing.

@robertodr robertodr closed this Nov 30, 2017
@robertodr robertodr deleted the runge_kutta_fehlberg78 branch April 26, 2018 22:44
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.

4 participants