Opened 5 years ago
Closed 4 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
- Owner changed from (none) to bhutz
comment:2 Changed 5 years ago by
- 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
- Status changed from new to needs_review
comment:4 Changed 5 years ago by
- Commit set to e43132a4054660d545733cbb2ab17dd56afbaf81
comment:5 Changed 4 years ago by
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 4 years ago by
- Status changed from needs_review to needs_info
comment:7 Changed 4 years ago by
- 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 4 years ago by
Oops, sorry, I was only looking at the second commit. Thanks for the clarification.
comment:9 Changed 4 years ago by
- Status changed from needs_review to positive_review
All tests passed on longtest
comment:10 Changed 4 years ago by
Reviewer name
comment:11 Changed 4 years ago by
- Reviewers set to Joao Alberto de Faria
comment:12 Changed 4 years ago by
- Branch changed from u/bhutz/ticket/15815 to e43132a4054660d545733cbb2ab17dd56afbaf81
- Resolution set to fixed
- Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
trac 15815: fixed doc test