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

Mv.__pow__ is not returning Mv instance as return value when raise to power of zero #518

Closed
mammalwong opened this issue May 11, 2024 · 5 comments
Assignees
Labels
bug component: core Ga, Mv, Metric, etc

Comments

@mammalwong
Copy link

mammalwong commented May 11, 2024

@utensil I have just realized there are another issue related to Mv.__pow__, in both primer-updates branch and version 0.5.1:

ga = Ga('e', g=[1,1,1], coords=S.symbols(f"0:{3}", real=True), wedge=False)
x,y,z = ga.mv()
(x**0).grade

I expect (x**0) returns ga.mv(1) and the .grade property will return [0]. However it raised this exception:

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
[<ipython-input-4-97f54e720c11>](https://localhost:8080/#) in <cell line: 3>()
      1 ga = Ga('e', g=[1,1,1], coords=S.symbols(f"0:{3}", real=True), wedge=False)
      2 x,y,z = ga.mv()
----> 3 (x**0).grade

AttributeError: 'One' object has no attribute 'grade'

Clearly it is because the method returning sympy.S.One instead of an Mv instance in this case

@mammalwong mammalwong changed the title Mv.__pow__ is not returning Mv class as return when raise to power of zero Mv.__pow__ is not returning Mv class as return value when raise to power of zero May 11, 2024
@mammalwong mammalwong changed the title Mv.__pow__ is not returning Mv class as return value when raise to power of zero Mv.__pow__ is not returning Mv instance as return value when raise to power of zero May 11, 2024
@utensil
Copy link
Member

utensil commented May 11, 2024

IIUC, this behavior is by design. In such cases, the result should downgrade to scalars naturally, so a Mv is no longer expected.

This is also a recurring theme during maintenance, where you could be dealing with a Mv then suddenly you are dealing with a SymPy expression like a Mul, or suddenly it's just a symbol that the result is simplified to.

This phenomenon is also responsible for the oversight you spot in #513 .

So, this is a non-issue for maintenance, though I agree with you on the UX aspect as a user.

@utensil utensil added wontfix component: core Ga, Mv, Metric, etc labels May 11, 2024
@mammalwong
Copy link
Author

mammalwong commented May 11, 2024

@utensil It sounds like a very weird design choice, I have multiple points to support that is weird:

  • lets say there is a non-scalar Mv instance $a$, when $a a$ is a scalar, a**2 returns a Mv instance of scalar value under the current implementation, while a**0 exceptionally return sympy.S.One instead of a Mv instance
  • in terms of math pow should be a mapping $Cl(p,q,r) \times Cl(p,q,r) \rightarrow Cl(p,q,r)$ even if the result is a scalar, that scalar should still have properties like its grade under the current Ga instance
  • in terms of coding it is a very error prone design, there are no documentation or return value type hint to indicate this exceptional case

utensil added a commit that referenced this issue May 11, 2024
@utensil utensil added bug and removed wontfix labels May 11, 2024
@utensil utensil self-assigned this May 11, 2024
@utensil
Copy link
Member

utensil commented May 11, 2024

For __pow__, a fix is pushed, and the existing tests are passing. So it would be fixed for this specific case, unless I find out that it causes other regressions. In general, this remains a wontfix, as you can find S.one usage everywhere in GAlgebra.

@mammalwong
Copy link
Author

Thanks for fixing this issue 👍

@utensil
Copy link
Member

utensil commented May 15, 2024

Thank you, @mammalwong , the fix should be in master now as #510 is now merged.

@utensil utensil closed this as completed May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component: core Ga, Mv, Metric, etc
Projects
None yet
Development

No branches or pull requests

2 participants