Skip to content

Conversation

fchapoton
Copy link
Contributor

@fchapoton fchapoton commented Oct 13, 2025

add type annotations to some of them

also change some of them to return Family objects or tuple

related to #41028 for Jordan algebras

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have created tests covering the changes.
  • [x I have updated the documentation and checked the documentation preview.

Copy link

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

cxzhong added a commit to cxzhong/sage that referenced this pull request Oct 14, 2025
@cxzhong
Copy link
Contributor

cxzhong commented Oct 14, 2025

LGTM. Thank you. Now it fixes the infinite loop bug in python 3.14

@cxzhong
Copy link
Contributor

cxzhong commented Oct 14, 2025

┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 10.8.beta6, Release Date: 2025-10-06              │
│ Using Python 3.14.0. Type "help()" for help.                       │
└────────────────────────────────────────────────────────────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Warning: this is a prerelease version, and it may be unstable.     ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
sage: F.<x,y,z> = FreeAlgebra(QQ)
sage: J = JordanAlgebra(F)
sage: J.gens()
Lazy family (Term map(i))_{i in Free monoid on 3 generators (x, y, z)}

It is the right output?

@fchapoton
Copy link
Contributor Author

yes, this is the expected output.

@user202729
Copy link
Contributor

user202729 commented Oct 14, 2025

Looking at the behavior, the reason for the change (test failure in Jordan algebra) is that in Python 3.14, tuple() no longer call __len__. So previously __len__ errors out, while now it just loop forever.

This change is caused by python/cpython#127758 which just modify the internal implementation of a function that converts from a sequence to a tuple.

vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 18, 2025
sagemathgh-41038: some care about algebra_generators
    
add type annotations to some of them

also change some of them to return Family objects or tuple

related to  sagemath#41028 for Jordan algebras

### 📝 Checklist

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have created tests covering the changes.
- [x I have updated the documentation and checked the documentation
preview.
    
URL: sagemath#41038
Reported by: Frédéric Chapoton
Reviewer(s):
@tscrim
Copy link
Collaborator

tscrim commented Oct 18, 2025

I am -1 on this change as the output of gens is expected to be a tuple (or at least behave like one).

@user202729
Copy link
Contributor

I am -1 on this change as the output of gens is expected to be a tuple (or at least behave like one).

there are infinitely many generators.

the current behavior (Python ≤ 3.13) is to raise an error, because tuple() internally call __len__ which raise an error and short circuit the next step. In Python 3.14, tuple() no longer call __len__.

Maybe you'd prefer raising an error (and maybe tell the user to look at .algebra_generators()) instead?

( @vbraun note that this pull request is un-positive-reviewed for now. I don't think you have automatic notification on status change after first positive review. )

@tscrim
Copy link
Collaborator

tscrim commented Oct 18, 2025

I am -1 on this change as the output of gens is expected to be a tuple (or at least behave like one).

Maybe you'd prefer raising an error (and maybe tell the user to look at .algebra_generators()) instead?

Yes, I think that proposed behavior is best. (There is also an argument for moving away from the ambiguous gens() too.) It's possible we are not consistent about gens() (I might even have been guilty of not doing this myself...), and this might be something we should discuss setting a standard for. I haven't checked this at all. However, I feel like we should try to have some consistency...

@user202729
Copy link
Contributor

user202729 commented Oct 18, 2025

in the case of J above, is J actually finitely generated as a Jordan algebra? It isn't obvious at all. (I suspect you can argue that it isn't, because suppose otherwise, J has a natural graded structure, there is a maximum degree of the generators, and you compare the dimension of the different levels or something)

On the other hand, we certainly are including some redundant entries in the list of algebra generators, for example

sage: import itertools
....: list(itertools.islice(J.algebra_generators(), 0, 10))
[1, x, y, z, x^2, x*y, x*z, y*x, y^2, y*z]
sage: J(x) * J(x)
x^2

so x^2 wouldn't be necessary to be included there.

@tscrim
Copy link
Collaborator

tscrim commented Oct 18, 2025

Yes, I agree that there are redundant generators, but this is allowed. It is not finitely generated for the $J(F_2)$ case. There is a natural (multi)grading on the Jordan algebra $J(A)$ inherited from the (multi)grading on the algebra $A$, which is clear from the definition of the product $x \circ y := (xy + yx)/2$. We can see that $j \circ j' = j' \circ j$ for any elements $j, j' \in J(A)$. Now suppose $F_2$ is the free algebra generated by $x,y$. I claim that we cannot find $x^k y$ for all $k &gt; 1$ if we are generated by $M_k$ all monomials of total degree $\leq k$. To see this, we consider the multigrading $(k, 1)$, which is $k+1$ dimension spanned by $\{x^k y, x^{k-1}yx, x^{k-2}yx^2, \ldots, yx^k\}$. So the only possible products we can get are

$$2 \cdot x^j \circ x^{k-j} y = x^ky + x^{k-j} y x^j \qquad\qquad (0 < j \leq k).$$

In other words, this is a spanning set for the Jordan subalgebra of $F_2$ generated (as a Jordan algebra) by $M_k$, which is only $k$ dimensional. Hence we are missing a basis element in this multidegree that needs to be a generator, and so $J(F_2)$ is infinitely generated.

Moreover, determining an exact generating set for $J(F_2)$ seems nontrivial (much less for the general case).

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.

4 participants