Skip to content

Conversation

@r-mb
Copy link
Contributor

@r-mb r-mb commented Jul 16, 2025

This PR adds an absolute_degree method to various finite fields class. This is done in order to fix is_prime_field. The reason is explained in my bug report #40426.

In the same time, I also updated documentation is various parts of the code.

Fixes #40426.

📝 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.

@r-mb r-mb force-pushed the quotient_is_prime_field branch from 7bd2c13 to 13bde43 Compare July 16, 2025 19:53
@user202729
Copy link
Contributor

To be fair, number fields have .degree(), .absolute_degree(), .relative_degree() too, with absolute degree being the degree over ℚ (the prime field in characteristic zero case).

Maybe we could have something similar here.

@r-mb
Copy link
Contributor Author

r-mb commented Jul 17, 2025

I agree that this would be the best solution. The thing is that, to have something similar to number fields, I believe we would either need to implement extensions of finite fields, which would require much work, or to do a breaking change.

Currently, SageMath uses the extension method of the CommutativeRing class, which in turn creates a PolynomialQuotientRing_field, which is a subclass of PolynomialQuotientRing_generic which defines degree.

If we rename degree there to relative_degree we would need to first do deprecation warnings isn't it? And then we could add an absolute_degree method in PolynomialQuotientRing_field and FiniteField.

@user202729
Copy link
Contributor

Not necessarily, we could just add absolute_degree which always mean degree over the prime field. Then modify is_prime_field to rely on absolute_degree.

@r-mb r-mb changed the title Add is_prime_field method to PolynomialQuotientRing_generic Add absolute_degree method to finite fields Jul 24, 2025
@r-mb
Copy link
Contributor Author

r-mb commented Jul 24, 2025

Sorry for the delay, I'm on holiday and will be slow to answer in the next weeks (but I will always reply eventually).

@user202729 I liked your idea to add absolute_degree very much, so I did so! This made me realise that the bug I fix in this PR also existed for extensions of QQ, so I also fixed that and added relevant doctests.

@r-mb
Copy link
Contributor Author

r-mb commented Jul 24, 2025

Failing doctests, please wait until I can fix this.

@r-mb
Copy link
Contributor Author

r-mb commented Jul 24, 2025

Ready for review!

@github-actions
Copy link

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

@user202729
Copy link
Contributor

user202729 commented Nov 21, 2025

I... guess I forgot this. Looks reasonable. I don't like the isinstance() and hasattr but there might not be a better way (duck typing).

(self in Fields() and self.absolute_degree()==1 might work, but what about something like Laurent polynomial ring which is a field of infinite degree over ℚ? Or say $ℚ_p$?)

@r-mb
Copy link
Contributor Author

r-mb commented Nov 24, 2025

Thank you!
Indeed having to do these checks is not the best, but by doing the other test we would need to implement absolute_degree everywhere, and make sure that every future new class of fields also do...

vbraun pushed a commit to vbraun/sage that referenced this pull request Nov 26, 2025
sagemathgh-40427: Add `absolute_degree` method to finite fields
    
This PR adds an `absolute_degree` method to various finite fields class.
This is done in order to fix `is_prime_field`. The reason is explained
in my bug report sagemath#40426.

In the same time, I also updated documentation is various parts of the
code.

Fixes sagemath#40426.

### 📝 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#40427
Reported by: Rubén Muñoz--Bertrand
Reviewer(s): Frédéric Chapoton, Rubén Muñoz--Bertrand
vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 1, 2025
sagemathgh-40427: Add `absolute_degree` method to finite fields
    
This PR adds an `absolute_degree` method to various finite fields class.
This is done in order to fix `is_prime_field`. The reason is explained
in my bug report sagemath#40426.

In the same time, I also updated documentation is various parts of the
code.

Fixes sagemath#40426.

### 📝 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#40427
Reported by: Rubén Muñoz--Bertrand
Reviewer(s): Frédéric Chapoton, Rubén Muñoz--Bertrand
@vbraun vbraun merged commit 7525734 into sagemath:develop Dec 2, 2025
21 of 23 checks passed
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.

False positive for is_prime_field for some finite field extensions

4 participants