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

Add nalgebra compatibility for Dual2 and Dual2Vec #81

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

RagibHasin
Copy link
Contributor

This implementation is in the same vein of #59.

Implementation of atan2 is inspired from HyperJet as @cormacrelf has noted before.

Basically it is a copy-paste job from existing implementation on Dual/DualVec with added atan2 from HyperJet.

@prehner
Copy link
Contributor

prehner commented Nov 5, 2024

Thank you for the contribution. As the discussion in #59 indicates, I am not a huge fan of implementing all this SIMD functionality into num-dual without actually using it. However, this is a criticism of how nalgebra handles trait bounds and not your work here. I am fine with merging it, but would you mind to elaborate a bit on how you benefit from this implementation? Is it linear algebra functionalities in nalgebra that are unlocked?

@RagibHasin
Copy link
Contributor Author

RagibHasin commented Nov 5, 2024

Yes. I only needed atan2(Dual2Vec, Dual2Vec) for a project of mine. As it is generalizable to any 2nd order dual numbers I made a PR.

@prehner
Copy link
Contributor

prehner commented Nov 14, 2024

In that case, we can also simply add atan2 to the DualNum trait. But since we have the nalgebra trait implementations already for the first order dual numbers, it makes sense to extend that to second order dual numbers. I'll merge that, but kind of really would prefer to get rid of the SIMD stuff for future maintenance. After all, we are not actually using SIMD to speed up calculations, are we? Only allowing the fields of the dual numbers to be SIMD values themselves, which I guess is already kind of cool. And of course we need it to enable nalgebra operations on matrices containing dual numbers (even though SIMD is probably not involved there in most cases).

@cormacrelf @tsmith023: would you mind sharing your opinion on this issue?

@prehner prehner merged commit 7e06070 into itt-ustutt:master Nov 14, 2024
5 checks passed
@RagibHasin RagibHasin deleted the nalgebra-dual2 branch November 14, 2024 11:15
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