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.

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

Random failure in enum_projective_number_field #17907

Closed
vbraun opened this issue Mar 7, 2015 · 12 comments
Closed

Random failure in enum_projective_number_field #17907

vbraun opened this issue Mar 7, 2015 · 12 comments

Comments

@vbraun
Copy link
Member

vbraun commented Mar 7, 2015

enum_projective_number_field (#17386) is missing points on OSX 10.6.8, possibly other platforms. This suggests that there is a bug in the algorithm where memory locations (determining sort order of objects without coercion into a common parent) matter.

  File "src/sage/schemes/projective/projective_rational_point.py", line 162, in sa  ge.schemes.projective.projective_rational_point.enum_projective_number_field 
  Failed example: 
    enum_projective_number_field(X(K),125) 
  Expected: 
    [(0 : 0 : 1), (-1 : -1 : 1), (1 : 1 : 1), (-1/5*v^2 : -1/5*v^2 : 1), (-v : -v : 1), 
    (1/5*v^2 : 1/5*v^2 : 1), (v : v : 1), (1 : 1 : 0)] 
  Got: 
    [(0 : 0 : 1), (-1 : -1 : 1), (1 : 1 : 1), (1 : 1 : 0)] 
********************************************************************** 

CC: @sagetrac-gjorgenson @bhutz @sagetrac-dkrumm @sagetrac-jdoyle

Component: algebraic geometry

Keywords: random_fail

Author: Ben Hutz

Branch: 4882838

Reviewer: Volker Braun

Issue created by migration from https://trac.sagemath.org/ticket/17907

@vbraun vbraun added this to the sage-6.6 milestone Mar 7, 2015
@bhutz
Copy link

bhutz commented Mar 7, 2015

comment:1

I haven't been able to reproduce this yet, but the issue is likely in #15389 since #17386 is just using that algorithm. So I've added John and David to the CC.

@sagetrac-jdoyle
Copy link
Mannequin

sagetrac-jdoyle mannequin commented Mar 7, 2015

comment:2

There seems to be an issue with the way 'bound' is defined for points_of_bounded_height. If the expected bound is an absolute height bound and you want to convert to a relative height bound, you need to raise to the power (degree of number field), not 1/(degree of number field). In particular, the expected output in this example contains points with relative height at most 5. For example, the point (2 : 2 : 1) doesn't appear in the expected output, though it certainly has height (relative and absolute) less than 125.

This doesn't address the discrepancy in the two lists, though, at least not entirely. The issue must be related to the floating point arithmetic being used for elements_of_bounded_height. The four points appearing in the "expected" list and not the "for" list have relative height exactly equal to 5, which is the bound that is being passed to the elements_of_bounded_height function. I seem to remember something like this happening when we were testing elements_of_bounded_height, where sometimes points of exact height B would appear in the output, and occasionally they would not, depending on the machine.

One last comment: Because of the floating point arithmetic involved, the output of elements_of_bounded_height cannot be guaranteed correct (as per the warning in the documentation). Probably a warning to this effect should be included in the documentation here.

@bhutz
Copy link

bhutz commented Mar 8, 2015

Branch: u/bhutz/ticket/17907

@bhutz
Copy link

bhutz commented Mar 8, 2015

Commit: 4882838

@bhutz
Copy link

bhutz commented Mar 8, 2015

comment:4

I've made a start here. I fixed the exponent on the bound. I added a precision parameter and the warning to the appropriate functions. I also upped the precision for the failed example, but since I can't reproduce, I can't be sure that will fix it.

I guess I'll mark this as needs-review, although I'm not 100% sure I've resolved it. I won't be able to work on this for a few days if someone wants to take over in the meantime.


New commits:

488283817907: fix precision issues in enum_projective_number_field

@bhutz
Copy link

bhutz commented Mar 8, 2015

Author: Ben Hutz

@vbraun
Copy link
Member Author

vbraun commented Mar 30, 2015

comment:5

Seems to fix the issue, at least.

Of course it would be nice to pick the right precision automatically ...

@vbraun
Copy link
Member Author

vbraun commented Mar 30, 2015

Reviewer: Volker Braun

@bhutz
Copy link

bhutz commented Mar 30, 2015

comment:6

Thanks for checking this. I've been having trouble finding someone with a machine which can reproduce the original error.

Yes, there is a TODO in the enumeration of points of bounded height in number fields (from #15389) to address the precision sensitivity.

@vbraun
Copy link
Member Author

vbraun commented Apr 14, 2015

Changed branch from u/bhutz/ticket/17907 to 4882838

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 18, 2015

Changed commit from 4882838 to none

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 18, 2015

Replying to @vbraun:

enum_projective_number_field (#17386) is missing points on OSX 10.6.8, possibly other platforms.

Just seen (with Sage 6.6) on Linux x86_64 (Haswell-E CPU), FSF GCC 4.9.2 (but not e.g. 4.4.3); patch fixes the doctest failure.

@nexttime nexttime mannequin modified the milestones: sage-6.6, sage-6.7 Apr 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants