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

Adding computation of complete elliptic integrals into amrex::Math so… #4151

Merged
merged 7 commits into from
Sep 18, 2024

Conversation

clarkse
Copy link
Contributor

@clarkse clarkse commented Sep 16, 2024

… that it can be executed on accelerator devices.

Summary

Currently the parser recognizes comp_ellint_1 and comp_ellint_2, but only executes on CPU when compiled with gcc. An implementation of the calculations have been added to amrex::Math to compute these quantities using the method described here:
https://dlmf.nist.gov/19.8

It is a quadratically converging iterative method that is compatible with device execution.

Gauss's Arithmetic-Geometric Mean
 

Additional background

Checklist

The proposed changes:

  • fix a bug or incorrect behavior in AMReX
  • add new capabilities to AMReX
  • changes answers in the test suite to more than roundoff level
  • are likely to significantly affect the results of downstream AMReX users
  • include documentation in the code and/or rst files, if appropriate

{
// Computing K based on DLMF
// https://dlmf.nist.gov/19.8
T tol = 1e-12;
Copy link
Member

Choose a reason for hiding this comment

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

What if T is float?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add a check on it to adjust the tolerance for a float to 1e-6.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be fixed now.

g0 = g;
}

return 0.5*pi<T>()/a;
Copy link
Member

Choose a reason for hiding this comment

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

If m is 0, a is uninitialized.

Copy link
Member

Choose a reason for hiding this comment

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

For comp_ellint_1(0.5), I get 1.685750355 with std::comp_ellint_1, but 1.854074677 with amrex::Math::comp_ellint_1. The error seems to be too big.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a definition issue. In most cases the integrals are defined with m as the argument. In this case with gcc it is k = sqrt(m). I will update it to be consistent and I will additionally add some test figures doing comparisons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@WeiqunZhang in general though do you think this is the right spot for this calculation?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think so.

Copy link
Member

Choose a reason for hiding this comment

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

It seems that we could use T tol = std::numeric_limits<T>::epsilon();. If we want to be conservative, we could make it 10x bigger than epsilon. In my testing, it only increases the number of iteration slightly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the latest push should resolve these concerns. I have also added a comparison of the AGM algorithm to what is done in Python's ellipm and ellipk routines.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The above figure is the difference between the two cases for the Z-components of the analytical solution to a current loop.

@WeiqunZhang WeiqunZhang merged commit 23a7f34 into AMReX-Codes:development Sep 18, 2024
59 checks passed
@clarkse clarkse deleted the add_gpu_comp_ellint branch September 18, 2024 02:42
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.

2 participants