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

Fix complex math abs and abs2 issues #3387

Conversation

sbastrakov
Copy link
Member

The issues were introduced in #3245 and went unnoriced until #3379 .

Fix wrong formula in Abs implementation.
Make return types of Abs and Abs2 real, as they were before that change.

Fixes #3379 .

@sbastrakov sbastrakov added bug a bug in the project's code component: PMacc in PMacc labels Oct 15, 2020
@sbastrakov sbastrakov added this to the 0.6.0 / 1.0.0: Next Stable milestone Oct 15, 2020
psychocoderHPC
psychocoderHPC previously approved these changes Oct 15, 2020
@psychocoderHPC
Copy link
Member

@BeyondEspresso PLease check if this solves you issues

@BeyondEspresso
Copy link
Member

For documentation, the PR created following compile error

/home/debus/src/picongpu/thirdParty/cupla/alpaka/include/alpaka/math/sqrt/Traits.hpp(51): error #70: incomplete type is not allowed
          detected during:
            instantiation of "auto alpaka::math::sqrt(const T &, const TArg &) [with T=alpaka::math::AbsUniformCudaHipBuiltIn, TArg=picongpu::float_X]" 
/home/debus/src/picongpu/include/pmacc/../pmacc/math/complex/Complex.tpp(224): here
            instantiation of "auto alpaka::math::traits::Abs<T_Ctx, pmacc::math::Complex<T_Type>, void>::abs(const T_Ctx &, const pmacc::math::Complex<T_Type> &)->T_Type [with T_Ctx=alpaka::math::AbsUniformCudaHipBuiltIn, T_Type=picongpu::float_X]" 
/home/debus/src/picongpu/thirdParty/cupla/include/cupla/device/math/Abs.hpp(37): here
            instantiation of "auto cupla::cupla_cuda_async::device::math::abs(const T_Type &)->decltype((<expression>)) [with T_Type=pmacc::math::Complex<picongpu::float_X>]" 
/home/debus/src/picongpu/include/pmacc/../pmacc/math/complex/Complex.tpp(182): here
            instantiation of "auto alpaka::math::traits::Sqrt<T_Ctx, pmacc::math::Complex<T_Type>, void>::sqrt(const T_Ctx &, const pmacc::math::Complex<T_Type> &)->pmacc::math::Complex<T_Type> [with T_Ctx=alpaka::math::SqrtUniformCudaHipBuiltIn, T_Type=picongpu::float_X]" 
/home/debus/src/picongpu/thirdParty/cupla/include/cupla/device/math/Root.hpp(37): here
            instantiation of "auto cupla::cupla_cuda_async::device::math::sqrt(const T_Type &)->decltype((<expression>)) [with T_Type=pmacc::math::Complex<picongpu::float_X>]" 
/home/debus/src/picongpu/include/pmacc/../picongpu/fields/background/templates/TWTS/EField.tpp(334): here

@sbastrakov
Copy link
Member Author

So I made a mistake in the fixed Abs implementation. Not sure what is it from the first look, will investigate tomorrow morning. Thanks for testing @BeyondEspresso , sorry it did not work as i imagined it would.



Fix wrong formula in Abs implementation.
Make return types of Abs and Abs2 real, as they were before that change.
@sbastrakov
Copy link
Member Author

@BeyondEspresso there was a tricky error in my original implementation. It is hopefully fixed (at least my small test compiled, but of course you have a more complex use case) now with the current state of this branch

@BeyondEspresso
Copy link
Member

BeyondEspresso commented Oct 16, 2020

@sbastrakov I ran my test case with your new branch and the results of the old master and your PR are virtually the same:
TWTSTest1
TWTSTest2

@psychocoderHPC psychocoderHPC merged commit 7a4ef86 into ComputationalRadiationPhysics:dev Oct 19, 2020
@sbastrakov sbastrakov deleted the fix-pmaccComplexNumberMath branch October 19, 2020 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug a bug in the project's code component: PMacc in PMacc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants