Opened 5 years ago

Closed 5 years ago

#15815 closed defect (fixed)

rational preimages for projective morphisms returns incorrect points

Reported by: bhutz Owned by: bhutz
Priority: minor Milestone: sage-6.2
Component: algebraic geometry Keywords:
Cc: Merged in:
Authors: Ben Hutz Reviewers: Joao Alberto de Faria
Report Upstream: N/A Work issues:
Branch: e43132a (Commits) Commit: e43132a4054660d545733cbb2ab17dd56afbaf81
Dependencies: Stopgaps:

Description

The function rational_preimages for projective morphism polynomials, returns points that are not pre-images of the input points. The issue is that the solutions found may only be solutions to some of the equations in the ideal.

I will add a check to insure only correct points are returned.

Change History (12)

comment:1 Changed 5 years ago by bhutz

  • Authors set to Ben Hutz
  • Owner changed from (none) to bhutz

comment:2 Changed 5 years ago by bhutz

  • Branch set to u/bhutz/ticket/15815
  • Created changed from 02/13/14 13:40:48 to 02/13/14 13:40:48
  • Modified changed from 02/13/14 14:12:10 to 02/13/14 14:12:10

comment:3 Changed 5 years ago by bhutz

  • Status changed from new to needs_review

comment:4 Changed 5 years ago by git

  • Commit set to e43132a4054660d545733cbb2ab17dd56afbaf81

Branch pushed to git repo; I updated commit sha1. New commits:

​e43132atrac 15815: fixed doc test

comment:5 Changed 5 years ago by chapoton

I do not see any relation between the ticket description and the actual content of the branch. And I do not understand how the modified test could even pass before the change in the branch.

comment:6 Changed 5 years ago by chapoton

  • Status changed from needs_review to needs_info

comment:7 Changed 5 years ago by bhutz

  • Status changed from needs_info to needs_review

Here is a longer description. The goal is to find all rational solutions to a zero dimensional ideal in projective space. The method is just basic elimination theory: compute an elimination ideal and solve one variable at a time. As each variable is solved the values are stored in points. The initial test was to check if there was an actual rational solution, i.e. we had a rational value for each of the N+1 variables. So to answer one question, the test would pass when there was a rational value for all N+1 variables.

The issue is that for higher dimensions there may be multiple equations involving one of the variables, one of which does and one of which does not have a rational solution. These slipped through to give wrong values. The test was modified to check that the "solution" really was a zero for *all* defining equations of the ideal.

As for the content of the branch matching the description. This seems clear to me. This is a modification to the {{rational_preimages}}} function to correct the particular problem I just described in more detail by adding the test I described. However, I did neglect to update it to say that I also added a simple codomain check to ensure that the base point was in the codomain of the function. Is that the point of confusion between description and branch?

If this is still not clear to you with this more verbose description, then I need a more specific question.

comment:8 Changed 5 years ago by chapoton

Oops, sorry, I was only looking at the second commit. Thanks for the clarification.

comment:9 Changed 5 years ago by jdefaria

  • Status changed from needs_review to positive_review

All tests passed on longtest

comment:10 Changed 5 years ago by vbraun

Reviewer name

comment:11 Changed 5 years ago by jdefaria

  • Reviewers set to Joao Alberto de Faria

comment:12 Changed 5 years ago by vbraun

  • Branch changed from u/bhutz/ticket/15815 to e43132a4054660d545733cbb2ab17dd56afbaf81
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.