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.

Sign up for GitHub

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

Jump to bottom

Jacobian arithmetic assertion error km-small model #39148

Closed
2 tasks done
vincentmacri opened this issue Dec 17, 2024 · 1 comment · Fixed by #39150
Closed
2 tasks done

Jacobian arithmetic assertion error km-small model #39148

vincentmacri opened this issue Dec 17, 2024 · 1 comment · Fixed by #39150
Labels
c: algebraic geometry t: bug

Comments

@vincentmacri
Copy link
Contributor

vincentmacri commented Dec 17, 2024 •
edited by kwankyu
Loading

Steps To Reproduce

While doing some testing of the km-small Jacobian model, I ran into this example which resulted in a failed assertion error when trying to add two elements.

k.<x> = FunctionField(GF(17))
t = polygen(k)

F.<y> = k.extension(t^4 + (14 * x + 14) * t^3 + 9 * t^2 + (10 * x^2 + 15 * x + 8) * t + 7 * x^3 + 15 * x^2 + 6 * x + 16)
x = F(x)
O = F.maximal_order()

infty1, infty2 = F.places_infinite()

P = O.ideal((x + 1, y + 7)).divisor()
D1 = 3 * infty2 + infty1 - 4 * P
D2 = F.divisor_group().zero()

# These are both Jacobian elements
assert D1.degree() == 0
assert D2.degree() == 0

J = F.jacobian(model='km-small', base_div = 4 * P)

JD1 = J(D1)
JD2 = J(D2)

JD1 + JD2  # AssertionError

Expected Behavior

Adding any two elements of the Jacobian should give a result

Actual Behavior

Adding these two elements gives an error

Additional Information

Tagging @kwankyu who added the Khuri-Makdisi Jacobian model in #35467.

The base divisor in the above example is 4*Place (x + 1, y + 7).

Additional pairs of elements that give an error when trying to add (same Jacobian as in the example above):

D1: 3*Place (1/x, 1/x*y)
 + Place (1/x, 1/x*y + 14)
 - 4*Place (x + 1, y + 7)
D2: 0

D1: 3*Place (1/x, 1/x*y)
 - 3*Place (x + 1, y + 7)
D2: Place (1/x, 1/x*y + 14)
 - Place (x + 1, y + 7)

D1: 2*Place (1/x, 1/x*y)
 + Place (1/x, 1/x*y + 14)
 - 3*Place (x + 1, y + 7)
D2: Place (1/x, 1/x*y)
 - Place (x + 1, y + 7)

D1: 2*Place (1/x, 1/x*y)
 - 2*Place (x + 1, y + 7)
D2: Place (1/x, 1/x*y)
 + Place (1/x, 1/x*y + 14)
 - 2*Place (x + 1, y + 7)

D1: Place (1/x, 1/x*y)
 + Place (1/x, 1/x*y + 14)
 - 2*Place (x + 1, y + 7)
D2: 2*Place (1/x, 1/x*y)
 - 2*Place (x + 1, y + 7)

D1: Place (1/x, 1/x*y)
 - Place (x + 1, y + 7)
D2: 2*Place (1/x, 1/x*y)
 + Place (1/x, 1/x*y + 14)
 - 3*Place (x + 1, y + 7)

D1: Place (1/x, 1/x*y + 14)
 - Place (x + 1, y + 7)
D2: 3*Place (1/x, 1/x*y)
 - 3*Place (x + 1, y + 7)

D1: 0
D2: 3*Place (1/x, 1/x*y)
 + Place (1/x, 1/x*y + 14)
 - 4*Place (x + 1, y + 7)

Environment

  • OS: Fedora 41
  • Sage Version: 10.5

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
@kwankyu kwankyu mentioned this issue Dec 17, 2024
Merged
5 tasks
@kwankyu
Copy link
Collaborator

kwankyu commented Dec 17, 2024

Thanks for reporting!

vbraun pushed a commit to vbraun/sage that referenced this issue Dec 19, 2024
sagemathgh-39150: Fix a bug in Khuri-Makdisi small model of Jacobian
894a5fe 
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Fixes sagemath#39148.

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->

URL: sagemath#39150
Reported by: Kwankyu Lee
Reviewer(s): Kwankyu Lee, Vincent Macri
vbraun pushed a commit to vbraun/sage that referenced this issue Dec 21, 2024
sagemathgh-39150: Fix a bug in Khuri-Makdisi small model of Jacobian
4e3005d 
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Fixes sagemath#39148.

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39150
Reported by: Kwankyu Lee
Reviewer(s): Kwankyu Lee, Vincent Macri
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: algebraic geometry t: bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix a bug in Khuri-Makdisi small model of Jacobian sagemath/sage
2 participants
@kwankyu @vincentmacri