Skip to content

new issue after moving the zeta method #40168

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

Open
2 tasks done
fchapoton opened this issue May 26, 2025 · 4 comments · May be fixed by #40170
Open
2 tasks done

new issue after moving the zeta method #40168

fchapoton opened this issue May 26, 2025 · 4 comments · May be fixed by #40170
Labels

Comments

@fchapoton
Copy link
Contributor

Steps To Reproduce

Some random failure in the CI can be reproduced by this :

sage: K=Integers(3659038768515778597)
sage: b=K(2734973578022464281)
sage: b.nth_root(3250669396930106370)
...
AttributeError: 'IntegerModRing_generic_with_category' object has no attribute '_element_of_factored_order'

Expected Behavior

either an answer or a ValueError

Actual Behavior

raise an AttributeError

Additional Information

No response

Environment

  • OS:
  • Sage Version:

Checklist

  • I have searched the existing issues for a bug report that matches the one I want to file, without success.
  • I have read the documentation and troubleshoot guide
@fchapoton
Copy link
Contributor Author

probably we need to implement a zeta method in IntegerMod rings ?

@tscrim
Copy link
Collaborator

tscrim commented May 27, 2025

I confirmed this was working in 10.7.beta3 but then failed when pulling in #39532.

@tscrim
Copy link
Collaborator

tscrim commented May 27, 2025

The problem comes from the fact that _element_of_factored_order is only implemented for a particular concrete implementation of FiniteField, not for the general category. When calling the nth_root(), it is doing a primality check and refining the category to the field:

sage: K=Integers(3659038768515778597)
sage: K.category()
Join of Category of finite commutative rings and Category of subquotients of monoids and Category of quotients of semigroups and Category of finite enumerated sets
sage: b=K(2734973578022464281)
sage: b.nth_root(3250669396930106370)
...
AttributeError: 'IntegerModRing_generic_with_category' object has no attribute '_element_of_factored_order'
sage: K.category()
Join of Category of finite enumerated fields and Category of subquotients of monoids and Category of quotients of semigroups

which subsequently leads to the failure.

Thus, to fix the issue, we need to lift the _element_of_factored_order up to the category. However, considering that the method has some (mild) Cython type declarations, I think we should leave the Cythonized copy there as it could have a performance impact. Of course, if it can be demonstrated that it has no real impact, then we can remove it.

@fchapoton
Copy link
Contributor Author

Thanks. I have made a proposal in #40170 by just copying the method in the category.

vbraun pushed a commit to vbraun/sage that referenced this issue May 28, 2025
sagemathgh-40170: generic implementation of _element_of_factored_order in finite-fields…
    
Fix sagemath#40168

### 📝 Checklist

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation and checked the documentation
preview.
    
URL: sagemath#40170
Reported by: Frédéric Chapoton
Reviewer(s): Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this issue May 29, 2025
sagemathgh-40170: generic implementation of _element_of_factored_order in finite-fields…
    
Fix sagemath#40168

### 📝 Checklist

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation and checked the documentation
preview.
    
URL: sagemath#40170
Reported by: Frédéric Chapoton
Reviewer(s): Travis Scrimshaw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants