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

Allow powers of SignGroup elements to live in other parents #38958

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

behackl
Copy link
Member

@behackl behackl commented Nov 12, 2024

A SignGroup is used as an auxiliary construct in the context of asymptotic expansions to split the growth of terms like (-2)^n * n^42 into, essentially, 2^n * n^42 * (-1)^n that lives in a cartesian product QQ^n * n^QQ * Signs^n.

In the current implementation of Sign.__pow__, the power is computed using the raw data (essentially, int ** whatever) and then an attempt is made to push the result back into SignGroup. This changes this behavior to still attempt the conversion -- but if it fails while the raw operation itself was successful, the result living in some other parent not coercing back to SignGroup is returned.

Simple example:

sage: from sage.groups.misc_gps.argument_groups import SignGroup
sage: S = SignGroup()
sage: x = SR.var('x')
sage: S(-1)^x  # this fails on develop
(-1)^x
sage: _.parent()
Symbolic Ring

For asymptotic expansions, this fixes an issue preventing substitutions in expansions with terms that involve, e.g., (-1)^n.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

Copy link

github-actions bot commented Nov 12, 2024

Documentation preview for this PR (built with commit c431e2c; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

P = self.parent()
return P.element_class(P, self._element_ ** exponent)
try:
result = P.element_class(P, self._element_ ** exponent)
Copy link
Contributor

Choose a reason for hiding this comment

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

why recompute the power ? and maybe use "return" in the try ?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are absolutely right, I changed the code in haste and missed replacing the argument. Fixed via c431e2c.

Copy link
Contributor

@fchapoton fchapoton left a comment

Choose a reason for hiding this comment

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

ok, looks good enough, although maybe not very kosher in term of the coercion model

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants