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

Improve perf of Legendre polynomial calculations #70

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

isaackunen
Copy link

The current code for calculating associated Legengre polynomials in G4LegendrePolynomial has two issues:

  1. It is a recursive solution to a recurrence whose time grows exponentially without a cache/memoization.

  2. The code can take a user-provided cache, but the user must be careful to supply a fresh cache for for each value of x or wrong results will be generated (silently).

This commit rewrites the G4LegendrePolynomial::EvalAssocLegendrePoly routine to address both of these issues:

  1. It iteratively walks up from P[m,m,x] to P[l,m,x] in O(l-m) time.

  2. It eliminates use of the cache entirely. A new signature without the cache has been introduced, and any call made with a cache falls through to this new one. The cache-bearing call has been retained in case any external callers rely on it.

  3. There do not appear to be any references to this method in the Geant4 codebase itself that pass a non-null cache. The few references that do exist are in G4PolarizationTransition; these have been rewritten to use the cache-free call.

  4. The list of special cases for small l and m values has been slightly expended (to negative m) and restructured as a case statement, primarily to improve readability. These have been retained since they appear to offer a modest performance benefit for these small cases.

This new method appears to perform better (often substantially better) than the old one over all tested parameters, even when a proper cache was used. It uses the same recurrence as the old method, and results seem identical.

The current code for calculating associated Legengre polynomials in
G4LegendrePolynomial has two issues:

1. It is a recursive solution to a recurrence whose time grows
   exponentially without a cache/memoization.

2. The code can take a user-provided cache, but the user must be careful
   to supply a fresh cache for for each value of x or wrong results will
   be generated (silently).

This commit rewrites the G4LegendrePolynomial::EvalAssocLegendrePoly
routine to address both of these issues:

1. It iteratively walks up from P[m,m,x] to P[l,m,x] in O(l-m) time.

2. It eliminates use of the cache entirely. A new signature without the
   cache has been introduced, and any call made with a cache falls
   through to this new one. The cache-bearing call has been retained
   in case any external callers rely on it.

3. There do not appear to be any references to this method in the Geant4
   codebase itself that pass a non-null cache. The few references that
   do exist are in G4PolarizationTransition; these have been rewritten
   to use the cache-free call.

4. The list of special cases for small l and m values has been slightly
   expended (to negative m) and restructured as a case statement,
   primarily to improve readability. These have been retained since they
   appear to offer a modest performance benefit for these small cases.

This new method appears to perform better (often substantially better)
than the old one over all tested parameters, even when a proper cache
was used. It uses the same recurrence as the old method, and results
seem identical.
@jasondet
Copy link

As the original author of G4LegendrePolynomial, I strongly support this. It should give a significant speed-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants