Opened 23 months ago

Closed 21 months ago

Last modified 20 months ago

#23193 closed enhancement (fixed)

some_elements is non-deterministic for function fields

Reported by: saraedum Owned by:
Priority: minor Milestone: sage-8.0
Component: commutative algebra Keywords: sd86.5, sd87, beginner
Cc: Merged in:
Authors: Hanson Smith, Julian Rüth Reviewers: Julian Rüth, Freda Li
Report Upstream: N/A Work issues:
Branch: 3d8c354 (Commits) Commit:
Dependencies: Stopgaps:

Description

This should return some "typical" elements that are good for testing. Just random elements is a bit unfortunate because we are missing elements such as 0, 1, x, 1/x, …

sage: K.<x> = FunctionField(QQ)
sage: K.some_elements()
[(x^2 - x - 1)/(-11/4*x^2 - x),
 (-x^2 - 1)/(-x - 1),
 (2/217*x^2 + 6*x - 3)/(-23*x^2 - 1/6*x - 2)]
sage: K.some_elements()
[-2*x - 1/7,
 (-x + 1)/(-2*x^2 + 2/3*x + 1/8),
 (x^2 + x - 1)/(-1/3*x^2 - 1/2*x - 35)]

Change History (18)

comment:1 Changed 23 months ago by hwsmith

  • Branch set to u/hwsmith/some_elements_is_non_deterministic_for_function_fields

comment:2 Changed 23 months ago by hwsmith

  • Authors set to Hanson Smith
  • Commit set to d9e145d44c3e2e18e00409ff93620d4bee6141a2
  • Status changed from new to needs_review

comment:3 Changed 23 months ago by git

  • Commit changed from d9e145d44c3e2e18e00409ff93620d4bee6141a2 to 59ecf9fce02b3507c037804a6a2b6f2c9a38eb14

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

59ecf9fNow, for function fields, some_elements gives a predetermined list of elements.

comment:4 Changed 23 months ago by git

  • Commit changed from 59ecf9fce02b3507c037804a6a2b6f2c9a38eb14 to faf01bc63d8f844e1f7d6fc384d080e1248083cd

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

faf01bcNow, for function fields, some_elements gives a predetermined list of elements.

comment:5 Changed 23 months ago by saraedum

Sorry, I said something incorrect about this earlier, claiming that this would only affect rational function fields. This actually gets called as well for extensions of function fields, so you can never simply divide by the generator or something like that sadly.

Here is an idea how you could probably elegantly rewrite this and do so without putting some random constants in the code here. self._ring is the polynomial ring underlying the function field (i.e., if you have K(x), then this would be K[x] and if you have K(x)[y]/(f), then this would be K(x)[y].) So you could just take numerators and denominators from this ring, i.e., do two nested for-loops (say a, b) that go through self._ring.some_elements(), check whether self(b) != 0 and if so add self(a)/self(b) to a list that you then return. What do you think?

You might want to also check that a != b and a != 0 to avoid duplicates in that list and then add [0,1] to the list manually.

comment:6 Changed 23 months ago by saraedum

  • Status changed from needs_review to needs_work

comment:7 Changed 23 months ago by saraedum

  • Reviewers set to Julian Rüth

comment:8 Changed 23 months ago by saraedum

  • Branch changed from u/hwsmith/some_elements_is_non_deterministic_for_function_fields to u/saraedum/some_elements_is_non_deterministic_for_function_fields

comment:9 Changed 22 months ago by git

  • Commit changed from faf01bc63d8f844e1f7d6fc384d080e1248083cd to 3d8c3546a27dc2d748eb8ca7059e758c498cfad9

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

3d8c354Return some_elements based on some_elements of a polynomial ring for function fields

comment:10 Changed 22 months ago by saraedum

  • Authors changed from Hanson Smith to Hanson Smith, Julian Rüth
  • Status changed from needs_work to needs_review

comment:11 Changed 21 months ago by saraedum

  • Keywords sd87 beginner added

comment:12 Changed 21 months ago by fli

  • Reviewers changed from Julian Rüth to Julian Rüth, Freda Li
  • Status changed from needs_review to positive_review

Doctests pass.

comment:13 Changed 21 months ago by vbraun

  • Branch changed from u/saraedum/some_elements_is_non_deterministic_for_function_fields to 3d8c3546a27dc2d748eb8ca7059e758c498cfad9
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:14 Changed 20 months ago by chapoton

  • Commit 3d8c3546a27dc2d748eb8ca7059e758c498cfad9 deleted

WARNING: this is causing the doctests of function_field to be VERY LONG (10 minutes) !

So please truncate this long list of some_elements to a much shorter list, until the doctest time is less than one minute.

See https://groups.google.com/forum/#!topic/sage-release/mSHCqsHpdkA

Last edited 20 months ago by chapoton (previous) (diff)

comment:15 Changed 20 months ago by saraedum

I guess there was no reason to delete the commit here? Ok. Let's fix this in a followup ticket.

comment:16 Changed 20 months ago by saraedum

Hm...but there seems to be no way of putting the commit back in.

comment:17 Changed 20 months ago by chapoton

The commit deletion is automatic when editing a close ticket, and should not be a problem.

comment:18 Changed 20 months ago by saraedum

I created #23683 to speed up function field tests.

Note: See TracTickets for help on using tickets.