Opened 8 years ago

Closed 8 years ago

#14015 closed enhancement (fixed)

Affine and Euclidean groups

Reported by: vbraun Owned by: joyner
Priority: major Milestone: sage-5.11
Component: group theory Keywords:
Cc: Merged in: sage-5.11.beta1
Authors: Volker Braun, Travis Scrimshaw Reviewers: Travis Scrimshaw, Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #14040, #14014 Stopgaps:

Description (last modified by tscrim)

This ticket implements basic affine groups and Euclidean groups:

sage: G = AffineGroup(3, QQ)
sage: g = G.random_element(); g
     [   2 -1/2    0]     [  0]
x|-> [   1   -1   -1] x + [-32]
     [   0   -2   -2]     [1/3]
sage: g*g
     [ 7/2 -1/2  1/2]     [   16]
x|-> [   1  5/2    3] x + [ -1/3]
     [  -2    6    6]     [191/3]
sage: g*g.inverse()
     [1 0 0]     [0]
x|-> [0 1 0] x + [0]
     [0 0 1]     [0]

Apply:

Attachments (2)

trac_14015_affine_group.patch (37.9 KB) - added by vbraun 8 years ago.
Initial patch
trac_14015-affine_groups-ts.patch (36.8 KB) - added by tscrim 8 years ago.

Download all attachments as: .zip

Change History (10)

Changed 8 years ago by vbraun

Initial patch

comment:1 Changed 8 years ago by vbraun

  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 8 years ago by vbraun

  • Dependencies set to #14040, #14014
  • Status changed from needs_review to needs_work

To be rebased on top of #14040, #14014.

Changed 8 years ago by tscrim

comment:3 Changed 8 years ago by tscrim

  • Description modified (diff)
  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_work to needs_review

Hey Volker,

Here's a rebased version with my review changes. I've removed the need for the *_generic classes and made the docstrings be at the class level so that they are visible using introspection. I've also added a method to get the lifted matrix space (representation of affine transformations as linear transformations) as linear_space(), and made a few docstring tweaks. If you're happy with my changes, you can set this to positive review.

Best,
Travis

For patchbot:

Apply: trac_14015-affine_groups-ts.patch

comment:4 Changed 8 years ago by vbraun

The _generic suffix was there so that we can later also wrap GAP's affine groups (especially for finite fields)

comment:5 Changed 8 years ago by tscrim

You can have the GAP's affine groups and have the __classcall__() return that class (see sage.combinat.partition.Partitions or sage.combinat.tableau.Tableaux as more complete/complicated examples). IMO this is cleaner since the we the class doesn't have any extra qualifiers, the (single) entry point matches the (base) class, and the classes have the correct naming scheme. Thus it is still extendable.

If the input format needs to be changed and exposed to the global namespace, you can implement a __classcall__() on the GAP wrapper parent (and likely the input will still need to standardized).

comment:6 Changed 8 years ago by vbraun

And you need some way to circumvent the enforced argument normalization for internal use where you know that the arguments don't have to be normalized. In terms of complexity / lines of code, I think its pretty much a draw. Which is to say, you end up using a lot of complicated machinery for no real advantage. And it gets even more complicated if you start deriving the class. And it breaks the symmetry between different implementations. If I had seen a real advantage with the __classcall__ mechanism then I would have used it myself.

comment:7 Changed 8 years ago by vbraun

  • Authors changed from Volker Braun to Volker Braun, Travis Scrimshaw
  • Milestone changed from sage-5.10 to sage-5.11
  • Reviewers changed from Travis Scrimshaw to Travis Scrimshaw, Volker Braun
  • Status changed from needs_review to positive_review

comment:8 Changed 8 years ago by jdemeyer

  • Merged in set to sage-5.11.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.