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

Deviation between MultiVector and MVArray calculation #426

Closed
joha2 opened this issue Jul 24, 2023 · 5 comments · Fixed by #437
Closed

Deviation between MultiVector and MVArray calculation #426

joha2 opened this issue Jul 24, 2023 · 5 comments · Fixed by #437

Comments

@joha2
Copy link

joha2 commented Jul 24, 2023

Hey guys!

First of all: Thank you very much for this library! It helped me a lot discovering GA 😄
I think I found a bug, because there is a discrepancy between the calculation with MultiVectors and with MVArrays. The chosen algebra is PGA in 3D with e0*e0 = 0.

My minimal working example code is the following:

import numpy as np
from clifford import Cl
import clifford

lpga3d, bpga3d = Cl(p=3, q=0, r=1, firstIdx=0)
Ipga = lpga3d.pseudoScalar

e0, e1, e2, e3 = lpga3d.basis_vectors.values()

print("Clifford version: ", clifford.__version__)
print("I = ", Ipga)
print("---------------")
a = e0 + 2*e1+3*e2+4*e3
aI = a*Ipga
scpaI = a|Ipga

print("a = ", a)
print("a*I = ", aI)
print("a|I = ", scpaI)
print("---------------")
b = np.ones(1, dtype=int)*(1*e0 + 2*e1 + 3*e2 + 4*e3)
bI = b*Ipga
scpbI = b|Ipga

print("b = ", b)
print("b*I = ", bI)
print("b|I = ", scpbI)
print("---------------")

The output is given by

Clifford version:  1.4.0
I =  (1^e0123)
---------------
a =  (1^e0) + (2^e1) + (3^e2) + (4^e3)
a*I =  -(4^e012) + (3^e013) - (2^e023)
a|I =  -(4^e012) + (3^e013) - (2^e023)
---------------
b =  [(1^e0) + (2^e1) + (3^e2) + (4^e3)]
b*I =  [-(4^e012) + (3^e013) - (2^e023)]
b|I =  [(4^e012) - (3^e013) + (2^e023)]
---------------

From an analytical point of view a^I = 0 since I has the highest grade.
I think the values for a are correct, but the scalar product b|I is wrong.

Is this a bug or did I do something wrong?

Best wishes
Johannes

@trundev
Copy link
Contributor

trundev commented Nov 22, 2024

The issue must be because of incorrect assumption that the inner product is commutative.
In _multivector.MultiVector, see this: __ror__ = __or__

I've isolated this by debugging the evaluation in both scenarios:

  • a|I (MultiVector | MultiVector) is evaluated via MultiVector.__or__
  • b|I (MVArray | MultiVector) is evaluated via MultiVector.__ror__, see operations with swapped operands

I.e. Python picks the MultiVector object in both cases, but since __or__ and __ror__ are the same, b|I is calculated like I|b.

If this is the case, I can create a change, to make code similar to __rxor__ vs. __xor__ (outter product).

Hope this could help
Todor

@joha2
Copy link
Author

joha2 commented Dec 8, 2024

Hey @trundev!

Thank you for your response. I just reported this bug, I did no further analysis where it comes from. I have the feeling, that clifford is not further maintained, since the last commit is long time ago.

For me, the bug is quite annoying, since I have a project, where I want to verify analytical calculations with the help of clifford and now I don't know how far I can trust it. It seems that you have found the cause of this bug here. So maybe, independently of the state of this repo, maybe you can fix it in your own fork and I can just switch to your fork.

Best wishes
Johannes

@arsenovic
Copy link
Member

hi!,
glad you all are using clifford, and sorry for your frustrations. clifford is for sure in need of a new maintainer. (#436)
if you make a PR for this change , i can merge it for you.

trundev added a commit to trundev/clifford that referenced this issue Dec 8, 2024
Add separate `MultiVector.__ror__` (based on `__or__`), to calculate
inner product from swapped operands. This is NOT commutative operation.
@trundev
Copy link
Contributor

trundev commented Dec 8, 2024

Hello, I just created a PR with the changes above: #437. Please let me know if this is done properly.

If you are interested I can suggest some more updates, like github workflow fixes and a fix for generated_jit issue 430.
The last one allows using of the latest numba and py 3.12. Please check my comment on that issue.

trundev added a commit to trundev/clifford that referenced this issue Dec 9, 2024
Add test to cover swapped operand operations on MultiVector-s.
Esp. the fix for pygae#426.
@joha2
Copy link
Author

joha2 commented Dec 28, 2024

@arsenovic and @trundev thanks for your help! My programs are now working as expected and in line with the analytical results! 😄

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 a pull request may close this issue.

3 participants