Coercion update
Robert Bradshaw
along. After a heroic effort by many people at Dev Days 1, and a
little followup work, we managed to merge the coercion branch into
3.0.3 and got all doctests to pass with the exception of /modular
and /schemes/generic. It has become apparent however that the
"coercion" branch contains much, much more than just coercion. We
also want to have the highest assurance of the integrity of the
resulting code which is hard to do with such a large set of
interdependent changes all lumped together as a single patch bomb.
Therefore we have decided to take the work we have so far and move it
over piece by piece. Specifically, we are inserting a new Parent
underneath the the old Parent object so we can migrate one thing at
at time. We will also be merging in the many not-directly related
improvements individually as separate tickets. The most significant
impact this will have is that it may take longer for the "new
coercion" to be 100% done, but the part that is done will be in Sage
itself, as well as being sanely refereed.
- Robert
Kurt
I'm afraid that I did not take detailed notes of your talk at Dev Days
1.
Is there documentation that shows how one ought to now implement
coercion for brand new code?
Perhaps a good example that we can all look to for guidance?
-- Kurt
On Jul 17, 11:25 pm, Robert Bradshaw <rober...@math.washington.edu>
wrote:
Robert Bradshaw
>
> Robert --
>
> I'm afraid that I did not take detailed notes of your talk at Dev
> Days 1.
> Is there documentation that shows how one ought to now implement
> coercion for brand new code?
> Perhaps a good example that we can all look to for guidance?
>
> -- Kurt
http://wiki.sagemath.org/days7/coercion/ is the most up to date,
which is to say not very. Documentation is certainly a high priority
for me now, though one *very* good thing that came out of Sage Dev
Days is that a lot more people understand the coercion model now. The
major push of this coercion rewrite (at least as it started) was to
make a better, more understandable API for people to use.
In terms of status, I got the new parent under the old parent (and
all doctests passing) and swapped out the coercion models (in the
process of writing a compatibility layer for old parents). When this
goes in, we can start writing against the new model. QQ, RR, CC, etc.
will probably be the best examples.
Robert Bradshaw
- Robert
William Stein
<robe...@math.washington.edu> wrote:
>
> Patches up at http://trac.sagemath.org/sage_trac/ticket/3738
>
Where is refereeing this? This needs to get refereed asap for obvious reasons,
so I'm curious who is going to do it.
-- William
Craig Citro
-cc
Robert Bradshaw
> I'm happy to volunteer starting Sunday, unless someone wants to
> beat me to it.
>
> -cc
Thanks.
>
> On Wed, Jul 30, 2008 at 6:05 AM, William Stein <wst...@gmail.com>
> wrote:
>>
>> On Tue, Jul 29, 2008 at 2:24 AM, Robert Bradshaw
>> <robe...@math.washington.edu> wrote:
>>>
>>> Patches up at http://trac.sagemath.org/sage_trac/ticket/3738
>>>
>>
>> Where is refereeing this? This needs to get refereed asap for
>> obvious reasons,
>> so I'm curious who is going to do it.
>>
>> -- William
I think this is probably something that it would be good for multiple
people could look at. (Both those previously involved in coercion,
and those not). It could help spread the work around too.
- Robert
William Stein
>
> I think this is probably something that it would be good for multiple
> people could look at. (Both those previously involved in coercion,
> and those not). It could help spread the work around too.
>
Michael Abshoff commented to me that the new coercion code you've
posted introduces new functions that have no doctests. Any comment?
-- William
Robert Bradshaw
That is a true accusation, though I feel there is some justification
as many of the functions are not actually being used yet so it is
hard to test them (they were being used in the coercion branch, so
it's not completely untested code, they're just not used until
Parents are converted.) Also, a lot of them are generic helper
functions that are supposed to be overridden. And a third point is
that much of this was written before the 100% doctest rule, and there
is a significant portion that is renaming/re-factoring old code.
That being said, I have strived for better coverage (and
documentation, not just tests). Currently in terms of new (or heavily
modified) code we have:
sage/structure/coerce.pyx
SCORE sage/structure/coerce.pyx: 100% (20 of 20)
sage/structure/coerce_actions.pyx
ERROR: Please define a s == loads(dumps(s)) doctest.
SCORE sage/structure/coerce_actions.pyx: 66% (10 of 15)
Missing documentation:
* __init__(self, G, S)
* Element _call_(self, g, a)
* __init__(self, G, S)
* Element _call_(self, a, g)
* __cinit__(self)
The first 4 are methods of the RAction and LAction classes that are
not yet used anywhere in the Sage library (but code exists to
instantiate them if _l_action or _r_action is defined on the
elements). I'm not sure if/how __cinit__ should be tested.
sage/structure/coerce_dict.pyx
SCORE sage/structure/coerce_dict.pyx: 100% (14 of 14)
sage/structure/coerce_maps.pyx
ERROR: Please define a s == loads(dumps(s)) doctest.
SCORE sage/structure/coerce_maps.pyx: 28% (7 of 25)
Mostly __init__ and _call_ methods, but since no Parents have been
converted over they are never used (and some can't even be used until
we have converted Parents). There still some room for improvement
here, and I will write some more documentation for this file.
Boilerplate __init__ functions are particularly unenlightening to
doctest.
sage/structure/generators.pyx
SCORE sage/structure/generators.pyx: 11% (5 of 45)
[we don't use this yet, other then the fact that there's a cdef'd
slot to put the generators object. If coverage on this file is a
problem, I would go ahead and delete much of this file, and only put
back things as they are needed]
That is where things stand. I will go add more doctests to
coerce_maps.pyx (though I know I can't hit 100% until it's actually
used) but I think it would be good to start reviewing it, and I would
advocate that for this particular project it be grandfathered in to
some extent for practicalities sake. (This is not the case for future
coercion tickets, which should meet the 100% coverage standard.)
- Robert
mabshoff
On Jul 30, 8:03 pm, Robert Bradshaw <rober...@math.washington.edu>
wrote:
> >> I think this is probably something that it would be good for multiple
> >> people could look at. (Both those previously involved in coercion,
> >> and those not). It could help spread the work around too.
>
> > Michael Abshoff commented to me that the new coercion code you've
> > posted introduces new functions that have no doctests. Any comment?
>
> That is a true accusation,
> though I feel there is some justification
> as many of the functions are not actually being used yet so it is
> hard to test them (they were being used in the coercion branch, so
> it's not completely untested code, they're just not used until
> Parents are converted.) Also, a lot of them are generic helper
> functions that are supposed to be overridden. And a third point is
> that much of this was written before the 100% doctest rule, and there
> is a significant portion that is renaming/re-factoring old code.
reasons you listed below. I was curious if we can get the coverage of
parent.pyx up to 100%, too?
> That being said, I have strived for better coverage (and
> documentation, not just tests). Currently in terms of new (or heavily
> modified) code we have:
>
> sage/structure/coerce.pyx
> SCORE sage/structure/coerce.pyx: 100% (20 of 20)
> sage/structure/coerce_actions.pyx
> ERROR: Please define a s == loads(dumps(s)) doctest.
> SCORE sage/structure/coerce_actions.pyx: 66% (10 of 15)
>
> Missing documentation:
> * __init__(self, G, S)
> * Element _call_(self, g, a)
> * __init__(self, G, S)
> * Element _call_(self, a, g)
> * __cinit__(self)
>
> The first 4 are methods of the RAction and LAction classes that are
> not yet used anywhere in the Sage library (but code exists to
> instantiate them if _l_action or _r_action is defined on the
> elements). I'm not sure if/how __cinit__ should be tested.
> sage/structure/coerce_dict.pyx
> SCORE sage/structure/coerce_dict.pyx: 100% (14 of 14)
> sage/structure/coerce_maps.pyx
> ERROR: Please define a s == loads(dumps(s)) doctest.
> SCORE sage/structure/coerce_maps.pyx: 28% (7 of 25)
>
> Mostly __init__ and _call_ methods, but since no Parents have been
> converted over they are never used (and some can't even be used until
> we have converted Parents). There still some room for improvement
> here, and I will write some more documentation for this file.
> Boilerplate __init__ functions are particularly unenlightening to
> doctest.
> sage/structure/generators.pyx
> SCORE sage/structure/generators.pyx: 11% (5 of 45)
>
> [we don't use this yet, other then the fact that there's a cdef'd
> slot to put the generators object. If coverage on this file is a
> problem, I would go ahead and delete much of this file, and only put
> back things as they are needed]
> That is where things stand. I will go add more doctests to
> coerce_maps.pyx (though I know I can't hit 100% until it's actually
> used) but I think it would be good to start reviewing it, and I would
> advocate that for this particular project it be grandfathered in to
> some extent for practicalities sake. (This is not the case for future
> coercion tickets, which should meet the 100% coverage standard.)
>
> - Robert
better make it very clear that we are facing a rather special
situation here and does not set a precedence for others who want to
bend the rules :)
Cheers,
Michael
Robert Bradshaw
> On Jul 30, 8:03 pm, Robert Bradshaw <rober...@math.washington.edu>
> wrote:
>> On Jul 30, 2008, at 9:49 AM, William Stein wrote:
>
> Hi Robert,
>
>>>> I think this is probably something that it would be good for
>>>> multiple
>>>> people could look at. (Both those previously involved in coercion,
>>>> and those not). It could help spread the work around too.
>>
>>> Michael Abshoff commented to me that the new coercion code you've
>>> posted introduces new functions that have no doctests. Any comment?
>>
>> That is a true accusation,
>
> It was merely an observation, not an accusation :)
>
>> though I feel there is some justification
>> as many of the functions are not actually being used yet so it is
>> hard to test them (they were being used in the coercion branch, so
>> it's not completely untested code, they're just not used until
>> Parents are converted.) Also, a lot of them are generic helper
>> functions that are supposed to be overridden. And a third point is
>> that much of this was written before the 100% doctest rule, and there
>> is a significant portion that is renaming/re-factoring old code.
>
> Yes, I think in this case we can make an exception for the various
> reasons you listed below. I was curious if we can get the coverage of
> parent.pyx up to 100%, too?
Not sure how doable that is, many of the functions here are meant to
be overridden by the user, or not called by the user (e.g. ones that
modify the state to set up coercion shouldn't be re-called), so it's
hard to test the functions themselves, especially the ones that
aren't used yet. I can certainly add some more doctests here though,
perhaps with the goal of at least exceeding the Sage total coverage.
Right now we are at
sage/structure/parent.pyx
ERROR: Please define a s == loads(dumps(s)) doctest.
SCORE sage/structure/parent.pyx: 19% (10 of 51)
where most of parent came (selectively) from
sage/structure/parent_old.pyx
ERROR: Please define a s == loads(dumps(s)) doctest.
SCORE sage/structure/parent_old.pyx: 4% (1 of 25)
sage/structure/parent_base.pyx
SCORE sage/structure/parent_base.pyx: 12% (1 of 8)
sage/structure/parent_gens.pyx
ERROR: Please define a s == loads(dumps(s)) doctest.
SCORE sage/structure/parent_gens.pyx: 21% (5 of 23)
For sure.
- Robert