Ticket #3999 (new defect)

Opened 3 months ago

Last modified 4 days ago

[with patch, needs work] Wrapper class to treat additive groups as multiplicative goups

Reported by: robertwb Assigned to: somebody
Priority: major Milestone: sage-3.2.2
Component: basic arithmetic Keywords:
Cc:

Description

This will greatly facilitate writing generic code.

        sage: from sage.groups.multiplicative_wrapper import MultiplicativeWrapper

        sage: R.<x,y> = ZZ[]
        sage: G = MultiplicativeWrapper(R)
        sage: a, b = G(x), G(y)
        sage: a^2 * b^5 * a
        (3*x + 5*y)
        sage: a/b
        (x - y)

        sage: E = EllipticCurve('37a')
        sage: P = E([0,0])
        sage: G = MultiplicativeWrapper(P.parent(), repr_format=None); G

        sage: a = G(P); a
        (0 : 0 : 1)
        sage: b = G(5*P); b
        (1/4 : -5/8 : 1)
        sage: a^2 * b
        (-5/9 : 8/27 : 1)
        sage: 7*P
        (-5/9 : 8/27 : 1)
        sage: 10*P == a^10
        True

Attachments

3999-multwrap.patch (10.2 kB) - added by robertwb on 08/30/2008 04:32:15 AM.

Change History

08/30/2008 04:30:53 AM changed by robertwb

Virtually no overhead:

sage: from sage.groups.multiplicative_wrapper import MultiplicativeWrapper
sage: R.<x,y> = ZZ[]
sage: G = MultiplicativeWrapper(R)
sage: f = R.random_element()
sage: g = R.random_element()
sage: timeit("2*f + 5*g")
625 loops, best of 3: 48.4 µs per loop
sage: a = G(f)
sage: b = G(g)
sage: timeit("a^2 * b^5")
625 loops, best of 3: 49 µs per loop

08/30/2008 04:32:15 AM changed by robertwb

  • attachment 3999-multwrap.patch added.

09/02/2008 12:32:27 PM changed by cremona

I am trying hard to see how this might actually be useful in practice.

If we had a whole implementation of additive abelian groups (as Z-modules, within the modules package) then this could be useful to allow for multiplicative abelian groups.

But in fact the whole abelian groups machinery is already multiplicative. What I tried, and nearly succeeded doing, was to all additive notation for those. Perhaps it would be possible to have a version of this patch which goes the other way. But I have other things to do than learn how to do that.

This is not a negative review, just a non-review.

09/02/2008 04:15:16 PM changed by robertwb

I am trying hard to see how this might actually be useful in practice.

The first thing that comes to mind is the generic discrete log code that you wrote a while back. One then wouldn't have to use the cumbersome (and slower) op(x,y) notation for the group operations to be able to handle both additive (via the wrapper) and multiplicative groups.

I had assumed the implementation of abelian groups was, at its core, additive using Z-modules and all (this seems more natural to me, as well as much more efficient). I'm not sure if this is part of the "great abelian groups rewrite" or not, but IMHO I think it should be. I also wrote the patch this direction because (in Sage) additive groups are all abelien (they inherit from modules), and there is a functor from them to generic non-abelien groups but not the other way around. It could also be handy in trying to (formally) implement Z[G] where G is initially presented as an additive group.

Trying to add additive notation to these would be difficult, as they do not inherit from ModuleElement?. Were you trying to make it so if I took elements of a multiplicative group (say, a permutation group) and did a+b it would instead do a*b. I would probably rather have it throw an error in this case.

I could pretty easily write a patch going the other way if you would find it useful (though strange stuff might happen if one tries to use it on non-abelien groups, depending on the assumptions people make throughout the rest of the Sage library).

11/28/2008 01:48:19 PM changed by was

  • summary changed from [with patch, needs review] Wrapper class to treat additive groups as multiplicative goups to [with patch, needs work] Wrapper class to treat additive groups as multiplicative goups.

REFEREE REPORT:

This bitrotted. I couldn't apply cleanly because of setup.py being refactored. I fixed that by hand, but then this wouldn't compile:

was@sage:~/build/sage-3.2.1.alpha1$ ./sage -br

----------------------------------------------------------
sage: Building and installing modified Sage library files.


Installing c_lib
scons: `install' is up to date.
Updating Cython code....
Building modified file sage/groups/multiplicative_wrapper.pyx.
Execute 1 commands (using 1 cpus)
python2.5 `which cython` --embed-positions --incref-local-binop -I/home/was/build/sage-3.2.1.alpha1/devel/sage-main -o sage/groups/multiplicative_wrapper.c sage/groups/multiplicative_wrapper.pyx

Error converting Pyrex file to C:
------------------------------------------------------------
...
        except:
            if x == 1:
                return self.one
            raise
            
    cpdef bint _has_coerce_map_from_(self, S) except -2:
         ^
------------------------------------------------------------

/home/was/build/sage-3.2.1.alpha1/devel/sage-main/sage/groups/multiplicative_wrapper.pyx:113:10: C method '_has_coerce_map_from_' not previously declared in definition part of extension type

Error converting Pyrex file to C:
------------------------------------------------------------
...
        cdef MultWrapperElement e = <MultWrapperElement>PY_NEW(MultWrapperElement)
        e._parent = self._parent
        e._elt = elt
        return e

    cdef MonoidElement _mul_c_impl(self, MonoidElement right):
        ^
------------------------------------------------------------

/home/was/build/sage-3.2.1.alpha1/devel/sage-main/sage/groups/multiplicative_wrapper.pyx:191:9: C method '_mul_c_impl' not previously declared in definition part of extension type

Error converting Pyrex file to C:
------------------------------------------------------------
...
        return e

    cdef MonoidElement _mul_c_impl(self, MonoidElement right):
        return self._new(self._elt._add_c((<MultWrapperElement>right)._elt))

    cdef MultiplicativeGroupElement _div_c_impl(self, MultiplicativeGroupElement right):
        ^
------------------------------------------------------------

/home/was/build/sage-3.2.1.alpha1/devel/sage-main/sage/groups/multiplicative_wrapper.pyx:194:9: C method '_div_c_impl' not previously declared in definition part of extension type
Parallel build failed with status 256.
sage: There was an error installing modified sage library code.