Skip to content

Conversation

@r-mb
Copy link
Contributor

@r-mb r-mb commented Oct 22, 2025

When creating a function field extension, it is never checked that the polynomial defining the extension is defined over that function field. Hence, when calling base_ring one gets a bug because the base ring of the polynomial ring is returned, and not the function field which we used to create the extension.

Fixes #41095.

When working on this, I remarked that the extension method of RationalFunctionField was just a copy paste (even for doctests) of the one of FunctionField, which is its parent. So I also removed it. But I can put it back if someone explains me why this can sometimes be useful to do so.

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

@github-actions
Copy link

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

EXAMPLES::
sage: K.<x> = FunctionField(QQ); R.<y> = K[]
sage: K.extension(y^5 - x^3 - 3*x + x*y) # needs sage.rings.function_field
Copy link
Contributor

@cxzhong cxzhong Oct 23, 2025

Choose a reason for hiding this comment

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

Now, we can not use K.extension like this? I think it is better to preserve these tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not understand, isn't it exactly line 456 in function_field.py? I feel like these tests were run twice, but maybe there is something silly I am missing?

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not understand, isn't it exactly line 456 in function_field.py? I feel like these tests were run twice, but maybe there is something silly I am missing?

The base field is QQ, a rational number field. But in line 456 examples, its base field is GF(2). This examples is used to detect the extension method on the function field based on a rational number field.

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not understand, isn't it exactly line 456 in function_field.py? I feel like these tests were run twice, but maybe there is something silly I am missing?

I think you can add this into the line456 test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I need another cup of black tea, I agree that the tests I added (starting line 475) are on GF(2), but it seems to me that the tests between line 456 and 469 are precisely the ones I deleted, with QQ. I just ran diff on these lines and the only difference is lines 463/464 vs 454/455. Maybe we can keep the small difference? But I don't see the GF(2) thing?

@cxzhong
Copy link
Contributor

cxzhong commented Oct 23, 2025

OK. Thank you.

vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 26, 2025
sagemathgh-41096: Fix bug with creation of extensions of function fields
    
When creating a function field extension, it is never checked that the
polynomial defining the extension is defined over that function field.
Hence, when calling `base_ring` one gets a bug because the base ring of
the polynomial ring is returned, and not the function field which we
used to create the extension.

Fixes sagemath#41095.

When working on this, I remarked that the `extension` method of
`RationalFunctionField` was just a copy paste (even for doctests) of the
one of `FunctionField`, which is its parent. So I also removed it. But I
can put it back if someone explains me why this can sometimes be useful
to do so.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [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#41096
Reported by: Rubén Muñoz--Bertrand
Reviewer(s): Chenxin Zhong, Rubén Muñoz--Bertrand
@vbraun vbraun merged commit 0b0b447 into sagemath:develop Oct 27, 2025
30 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.

Wrong base ring returned with multiple function fields extension

3 participants