Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#15393 closed enhancement (fixed)

compute the automorphism group of a morphism of P1

Reported by: bhutz Owned by: bhutz
Priority: major Milestone: sage-6.4
Component: algebraic geometry Keywords: sage-days55
Cc: Merged in:
Authors: Ben Hutz, Bianca Thompson, Joao Alberto de Faria Reviewers: Grayson Jorgenson
Report Upstream: N/A Work issues:
Branch: 58129a6 (Commits) Commit:
Dependencies: Stopgaps:

Description

Faber-Manes-Viray published an algorithm to compute the automorphism group of a morphism of P1, "Computing conjugating sets and automorphism groups of rational functions."

Translate their code to work with the existing frame work of dynamics in Sage.

Attachments (1)

trac_15393_automorphism_group.patch (78.4 KB) - added by bhutz 4 years ago.
fixed some typos

Download all attachments as: .zip

Change History (29)

comment:1 Changed 4 years ago by bhutz

  • Authors set to bhutz
  • Status changed from new to needs_review

We've modified (and documented) the FMV automorphism group algorithms to fit into the implementations in Sage for morphisms of projective space. There are a fair number of changes to the original code, but the algorithms are unchanged.

comment:2 Changed 4 years ago by bhutz

updated the filename to be more distinctive

Changed 4 years ago by bhutz

fixed some typos

comment:3 Changed 4 years ago by bthompson

  • Authors changed from bhutz to bhutz, bthompson

comment:4 Changed 4 years ago by mmarco

Just a quick comment: there should be doctests that show the iso_type option.

comment:5 Changed 4 years ago by bhutz

Actually there are. See lines 1856 and 2802. Those are functions that are exposed to the user.

comment:6 Changed 4 years ago by jdefaria

  • Authors changed from bhutz, bthompson to bhutz, bthompson, jdefaria

comment:7 Changed 4 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:8 Changed 3 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:9 Changed 3 years ago by bhutz

  • Branch set to u/bhutz/ticket/15393
  • Commit set to 88ad7deca2fadbb8b70ed82baf43f61e748d78d1

moved patch to branch


New commits:

88ad7detrac 15393: imported patch to branch

comment:10 Changed 3 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:11 Changed 3 years ago by gjorgenson

  • Reviewers set to Grayson Jorgenson
  • Status changed from needs_review to needs_work

Reviewed code and came up with a list of minor changes and some issues with the functionality:

  • Add comment before use of Subset stating that unordered subset selection works for QQ
  • In the rational fixed points function, add example demonstrating the iso_type parameter
  • Elaborate on the set W referenced from ln 134 onwards in comments
  • ln 126, 259, change ZZ to F
  • ln 237, 253, 269, change K(s) to s when appending
  • ln 287: 'We have proved that to In [FMV] it is proven that'
  • ln 332: remove "an automorphism of P1"
  • ln 385: B=copy(A)
  • ln 394: remove 'multiplicative'
  • remove 'get_orders'. Change 'PGL_order' to be 'order' and have it call 'PGL_repn'. Fix ln 775 where 'getorders' is called (don't forget to sort by order)
  • import 'CRT' -> 'from sage.rings.arith import CRT'
  • ln 464: Integers in not imported
  • ln 498: describe 'moduli' as integer prime powers
  • in CRT_automorphisms: define M (see CRT_helper ln 430). Output also includes M
  • 'd' is not needed
  • 'CRT_automorphisms' can pass an empty list ot CRT_helper if there are no degree 'd' automorphisms. If it does encounter an empty list it needs to not pass it (or its moduli) to CRT_helper.open (maybe not since the map is injective)
  • ln 521: 'automorphim'
  • ln 534: ending comma not needed. add defaults. add that automorphisms are matrices. Deleted commented out code
  • ln 532: 'M' is an integer (product of prime powers)
  • ln 563: comment explaining 'scalar*a - (scalar*a/M).round()*M' instead of (scalar*a).lift()
  • ln 568: condense 'if' use max(abs(aa),abs(bb),abs(cc),abs(dd)), from previous change don't need absolute value.
  • ln 664: comment about del needs to start at the end not to have issues

automorphism_group_QQ_CRT

  • defaults for keywords
  • OUTPUT: remove 'as linear fraction transformations'
  • line 735: h=h/(gcd(h,h.derivative())), Actually do this as done on line 1260
  • line 738: points -> point
  • line 766 Q -> QQ
  • line 772: not needed comment line
  • line 788-789: A or [B and (C1 or C2)], missing []
  • line 797: QQ
  • line 805: extra line
  • line 815: extra line
  • line 827: QQ
  • line 178, 851, 852: k in never incremented
  • CRT does not seem to actually be used very well. It checks all lifts for one prime (which sine the reduction map is injective should find all the maps). This is highly inefficient when the height bound is high.

automorphism_group_FF

  • remove authors part to top of file
  • algoritm -> use citation
  • default for return_functions
  • examples, add keywords
  • line 920 938, 941 extra line

  • line 948: determines
  • line 949 yy has wrong number quotes
  • 972: no need for y
  • 983: X is not used
  • 1006: use sigma not ssigma, delete 1045
  • 1057: just use poly_ring not R
  • 1052-1053: why scaling to have constant term of numerator be 1
  • 1059-60 could use poly constructor R(ff), R(gg)
  • 1131: use citation
  • 1149: no 'phi' no 'E'
  • line 1205 extra line
  • line 1215 no 'F'
  • line 1268, 1275, 1324 extra line
  • order_p_automorphism, cite algorithm 4 in FMV
  • 1396 means we don't need pre_data
  • extra line 1451, 1462, 1474, 1485, 1489, 1501
  • line 1523 - Boolean type
  • line 1527: automorphisms with order prime to $p$.
  • 1549 get rid of 'T'
  • 1559 - assume 2nd coordinate of point is 1 (i.e. normalized
  • extra line 1565
  • line 1568: sqrt to get cardinality of the field and not the degree 2 extension
  • line 1595 : citation
  • line 1675: remove ???
  • line 1760-1765 delete
  • line 1797 should raise and error
Last edited 3 years ago by gjorgenson (previous) (diff)

comment:12 Changed 3 years ago by git

  • Commit changed from 88ad7deca2fadbb8b70ed82baf43f61e748d78d1 to 7bdf289139a6ea011ede1f3c8fcd68ea9a83928a

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

7bdf28915393: fixed numerous small issues from review

comment:13 Changed 3 years ago by gjorgenson

The following map fails:

sage: R.<x,y> = ProjectiveSpace(QQ,1)
sage: H = End(R)
sage: f=H([6*x^2 - 39*y^2, 2*y^2])
sage: f.automorphism_group(algorithm='CRT',return_functions=True, iso_type=True)
AttributeError: 'sage.rings.integer.Integer' object has no attribute
'polynomial'

comment:14 Changed 3 years ago by git

  • Commit changed from 7bdf289139a6ea011ede1f3c8fcd68ea9a83928a to f7ee9c2fc1f86f223d5183c4fc197055c3749736

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

f7ee9c215393: resolved remaining issues

comment:15 Changed 3 years ago by bhutz

  • Authors changed from bhutz, bthompson, jdefaria to Ben Hutz, Bianca Thompson, Joao de Faria
  • Status changed from needs_work to needs_review

Working with Xander Faber, remaining issues are resolved.

comment:16 Changed 3 years ago by git

  • Commit changed from f7ee9c2fc1f86f223d5183c4fc197055c3749736 to ab8d71a96f2eff9145e5b25076ef495e1ba122b4

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

ab8d71a15393: fix minor typos

comment:17 Changed 3 years ago by gjorgenson

  • Status changed from needs_review to positive_review

Everything seems good.

comment:18 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

Doctests fail after #17518

comment:19 Changed 3 years ago by git

  • Commit changed from ab8d71a96f2eff9145e5b25076ef495e1ba122b4 to 84e59cf5387e61b340081310ed17af1575d0afb9

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

84e59cf15393: updated coeffs for #15718

comment:20 Changed 3 years ago by bhutz

Well, that should fix it, but I can't seem to download 17518 to verify. I get

The branch field on ticket #17518 is set to the non-existent
"bb4a957ea6838b11b8ec8162dc657e215dfa3dc5". Please set the field on trac to a field value.

If someone could tell me how to download this branch for the closed 17518, I can test. Otherwise, I guess I'll need to wait until the beta including 17518 comes out.

comment:21 Changed 3 years ago by bhutz

never mind, I see it is in beta5. building, then I'll check the tests

comment:22 Changed 3 years ago by git

  • Commit changed from 84e59cf5387e61b340081310ed17af1575d0afb9 to 7804a79595fa3a76ca8aaae40c68fe04d68fd7d7

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

7804a7915393: fix doc test formatting

comment:23 Changed 3 years ago by git

  • Commit changed from 7804a79595fa3a76ca8aaae40c68fe04d68fd7d7 to 3ccbe1bae0878619e58983f3dd70cc56a0358663

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

3ccbe1b15393: minor doc fix

comment:24 Changed 3 years ago by bhutz

  • Status changed from needs_work to needs_review

comment:25 Changed 3 years ago by git

  • Commit changed from 3ccbe1bae0878619e58983f3dd70cc56a0358663 to 58129a61395c9d7b8681c9097eeb5db3bb1b0a10

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

58129a6Merge branch 'master' into ticket/15393

comment:26 Changed 3 years ago by gjorgenson

  • Status changed from needs_review to positive_review

Documentation looks good. Doctests and tests on example functions passed.

comment:27 Changed 3 years ago by vbraun

  • Branch changed from u/bhutz/ticket/15393 to 58129a61395c9d7b8681c9097eeb5db3bb1b0a10
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:28 Changed 3 years ago by jdemeyer

  • Authors changed from Ben Hutz, Bianca Thompson, Joao de Faria to Ben Hutz, Bianca Thompson, Joao Alberto de Faria
  • Commit 58129a61395c9d7b8681c9097eeb5db3bb1b0a10 deleted
Note: See TracTickets for help on using tickets.