Make getattr faster on parents and elements

[simon-king-jena]

If an attribute of a parent or element can not be found by inspecting the method resolution order, __getattr__ is invoked. It tries to obtain the attribute from the parent class or the element class of the category of the parent. If it can not be found, the attribute error is raised using a Cython function sage.structure.parent.raise_attribute_error.

I see several ways to make both __getattr__ and sage.structure.parent.raise_attribute_error a lot faster. Details will be given in the comments.

Because of one doctest in sage.categories.primer, it is needed that the category of polynomial rings is properly initialised. Therefore:

Depends on #9944

Apply

• attachment: trac11342-attribute_error_message.rebased.patch
• attachment: trac_11342_fix_pari_initialization.patch
• attachment: trac_11342_fix_pari_ring.patch
• attachment: trac_11342_crypto_fixes.patch
• attachment: trac11342_test_parent_on_getattr.patch

Depends on #9944

CC: @jdemeyer

Component: performance

Keywords: getattr parent element

Author: Simon King, Volker Braun

Reviewer: Jeroen Demeyer, Volker Braun, Simon King

Merged: sage-4.7.2.alpha3

Issue created by migration from https://trac.sagemath.org/ticket/11342

[simon-king-jena]

Attachment: trac11342-faster_getattr.patch.gz

Make attribute access faster on elements and parents

[simon-king-jena]

comment:1

Here are my ideas to make attribute access faster.

1. Make raising an attribute error faster

It is very common that one tries to get an attribute, but it does not exist - attribute error. Since it occurs so frequently, it is essential that the error be raised as quickly as possible. Raising the error is therefore the job of a Cython function raise_attribute_error in sage.structure.parent. However, that function can be slightly improved.

1.a) The second argument to that function always is a string: The attribute name. Thus, why not explicitly state it in the Cython code?

1.b) It internally operates with the type of the first argument. So, why not explicitly state it in the Cython code?

1.c) The error message is formed from the name of the class and the name of the attribute. There is one complication: If one has an extension type then its name shall be prepended by its module name. There is a Cython function is_extension_type for telling whether it is an extension type or not. But the function is small, and a tiny bit of time can be saved by doing the test inside raise_attribute_error.

1.d) How shall one create the error message? In unpatched Sage, it is done by a formatted string that is formed by the class name (perhaps together with the name of the module of the class) and the attribute name. However, it turned out in some timings that I did that composing the error message by addition of strings is faster than the formatted string.

1.e) If one has an extension type then it turns out to be faster to get module name plus class name from the string representation of that type, rather than from addition of strings.

Hence, the new raise_attribute_error looks like this:

cdef inline raise_attribute_error(self, str name):
    cdef type cls = type(self)
    cdef int dictoff
    try:
        dictoff = cls.__dictoffset__
    except AttributeError:
        raise AttributeError, "'"+cls.__name__+"' object has no attribute '"+name+"'"
    if dictoff:
        raise AttributeError, "'"+cls.__name__+"' object has no attribute '"+name+"'"
    raise AttributeError, repr(cls)[6:-1] + " object has no attribute '"+name+"'"

Timings for raise_attribute_error

I compare the timings of unpatched and patched sage-4.7.alpha5, stating the patched version first. We have to distinguish extension types and others:

sage: cython("""
....: from sage.structure.parent cimport raise_attribute_error
....: def test(self, name, long m):
....:     cdef long i
....:     for i from 0<=i<m:
....:         try:
....:             raise_attribute_error(self,name)
....:         except AttributeError:
....:             pass
....: """)
sage: P.<a,b,c> = PolynomialRing(QQ)
sage: P.__class__.__dictoffset__
0
sage: QQ.__class__.__dictoffset__
288
sage: %time test(P,'bla',10^7)
CPU times: user 19.99 s, sys: 0.00 s, total: 19.99 s
Wall time: 19.99 s
# unpatched:
# CPU times: user 21.28 s, sys: 0.00 s, total: 21.28 s
# Wall time: 21.28 s
sage: %time test(QQ,'bla',10^7)
CPU times: user 17.66 s, sys: 0.01 s, total: 17.67 s
Wall time: 17.67 s
# unpatched:
# CPU times: user 19.92 s, sys: 0.01 s, total: 19.93 s
# Wall time: 19.93 s

2. Raise attribute errors earlier

An attribute of a parent can be obtained from the category only if its category is properly initialised. Otherwise an attribute error is raised. In order to test whether it is properly initialised, the method _is_category_initialized is called. However, that method merely tests whether the cdef attribute _category of the parent is None.

Hence, first suggestion: Directly test whether self._category is None, if self is a parent.

Up to now, an attribute of an element can be obtained from the category of its parent, even if the category of the parent is not properly initialised. I doubt that this is a good idea. In fact, there is only a single doctest error if one requests the parent's category to be initialised, and this error vanishes if #9944 is applied.

Hence, second suggestion: If self is an element, then try to get an attribute from the category only if self._parent._category is not None. These changes have a nice effect on the performance. Again, I state the timings for the patched version first.

Recall that by #10467, if the __getattr__ method of a parent (or element) is asked for a private attribute (one that starts with double underscore but does not end with an underscore), it immediately raises an error, since by convention private attributes can not be inherited from the category (or they wouldn't be private).

Hence, we have to consider the following cases:

1. non-existing private attributes

sage: a = 1
sage: timeit('try: QQ.__bla\nexcept: pass')
625 loops, best of 3: 13.8 µs per loop
# unpatched: 625 loops, best of 3: 15.3 µs per loop
sage: timeit('try: a.__bla\nexcept: pass')
625 loops, best of 3: 13.3 µs per loop
# unpatched: 625 loops, best of 3: 14 µs per loop

2. non-existing non-private attributes

sage: timeit('try: QQ.__bla_\nexcept: pass')
625 loops, best of 3: 17.2 µs per loop
# unpatched: 625 loops, best of 3: 23.4 µs per loop
sage: timeit('try: a.__bla_\nexcept: pass')
625 loops, best of 3: 21.6 µs per loop
# unpatched: 625 loops, best of 3: 28.2 µs per loop
sage: timeit('try: QQ.bla\nexcept: pass')
625 loops, best of 3: 16.7 µs per loop
# unpatched: 625 loops, best of 3: 22.9 µs per loop
sage: timeit('try: a.bla\nexcept: pass')
625 loops, best of 3: 20.8 µs per loop
# unpatched: 625 loops, best of 3: 27.2 µs per loop

3. existing attributes inherited from the category

sage: timeit('try: QQ.sum\nexcept: pass',number=10^5)
100000 loops, best of 3: 740 ns per loop
# unpatched: 625 loops, best of 3: 744 ns per loop
sage: timeit('try: a.cartesian_product\nexcept: pass',number=10^5)
100000 loops, best of 3: 1.67 µs per loop
# unpatched: 100000 loops, best of 3: 3.89 µs per loop

Long doctests pass for me - #9944 is required for one test in sage.category.primer. So: Needs review!

Depends on #9944

[simon-king-jena]

Dependencies: #9944

[simon-king-jena]

Author: Simon King

[simon-king-jena]

Make attribute access faster on elements and parents: Create the error message only when needed

[simon-king-jena]

comment:2

Attachment: trac11342-AttributeErrorMessage.patch.gz

Using an additional idea, I was able to improve the timings even further.

I have already indicated that the process of raising the attribute error actually is a bottle neck. To be precise: Most time is spent for creating the nicely formatted error message.

The point is: The typical fate of an AttributeError is being caught. Hence, nobody will ever see the error message, except under very special circumstances.

Therefore I suggest that the error message will only be created if someone wants to see it. This is possible by introducing a new class sage.structure.parent.AttributeErrorMessage. Its instances simply store a class cls and an attribute name, and its string representation is the well-known error message stating that cls has no attribute of the given name. This string representation is not created during initialisation but only when needed.

Hence, rather than calling raise_attribute_error(self,name), one would do raise AttributeError, AttributeErrorMessage(self,name). That is a lot faster:

sage: cython("""
....: from sage.structure.parent import AttributeErrorMessage
....: def test(self,name,long m):
....:     cdef long i
....:     for i from 0<=i<m:
....:         try:
....:             raise AttributeError, AttributeErrorMessage(self,name)
....:         except AttributeError:
....:             pass
....: """)
sage: P.<a,b,c> = PolynomialRing(QQ)
sage: %time test(P,'bla',10^7)
CPU times: user 3.10 s, sys: 0.00 s, total: 3.10 s
Wall time: 3.10 s
sage: %time test(QQ,'bla',10^7)
CPU times: user 3.06 s, sys: 0.00 s, total: 3.06 s
Wall time: 3.07 s

Compare with the timings for raise_attribute_error in my previous comments!

Other than that, I added the specification that the attribute name is supposed to be a string:

def __getattr__(self, str name):

instead of

def __getattr__(self, name):

Updated timings

I am repeating the tests from my previous comments.

1. non-existing private attributes

There is a clear improvement, even compared with the previous patch version.

sage: a = 1
sage: timeit('try: QQ.__bla\nexcept: pass')
625 loops, best of 3: 10.5 µs per loop
sage: timeit('try: a.__bla\nexcept: pass')
625 loops, best of 3: 8.78 µs per loop
sage: timeit('try: QQ.__bla_\nexcept: pass')

2. non-existing non-private attributes

There is a clear improvement, even compared with the previous patch version.

sage: timeit('try: QQ.__bla_\nexcept: pass')
625 loops, best of 3: 14.1 µs per loop
sage: timeit('try: a.__bla_\nexcept: pass')
625 loops, best of 3: 16.1 µs per loop
sage: timeit('try: QQ.bla\nexcept: pass')
625 loops, best of 3: 13.8 µs per loop
sage: timeit('try: a.bla\nexcept: pass')
625 loops, best of 3: 16.3 µs per loop

3. existing attributes inherited from the category

sage: timeit('try: QQ.sum\nexcept: pass',number=10^5)
100000 loops, best of 3: 741 ns per loop
sage: timeit('try: a.cartesian_product\nexcept: pass',number=10^5)
100000 loops, best of 3: 1.67 µs per loop

Note

Usually, errors are raised with an error message that is given by a string. It seems that raising it with an instance of AttributeErrorMessage is fine as well - at least it does not seem to break existing tests. However, perhaps other people have objections.

Therefore I'm informing sage-devel, and I did not override the old patch but created a new one.

For the patchbot:

Depends on #9944

Apply [attachent:trac11342-AttributeErrorMessage.patch]

[hivert]

comment:3

Hi Simon,

I didn't realize that it was part of your problem, that's why I only react now. It seems that your class AttributeErrorMessage somehow reproduce the feature of LazyFormat (see #11342). We probably should merge the two features. Note that mine is pure Python so that probably a little Cythonizing is needed if it fits you needs.

Florent

[simon-king-jena]

comment:4

Replying to @hivert:

I didn't realize that it was part of your problem, that's why I only react now. It seems that your class AttributeErrorMessage somehow reproduce the feature of LazyFormat (see #11342).

You mean #8742.

Thank you for the pointer to lazy format strings!

[simon-king-jena]

comment:5

I did the following test with lazy format strings:

sage: cython("""
....: from sage.misc.lazy_format import LazyFormat
....: def test(self,name,long m):
....:     s = LazyFormat("%s has no attribute %s")
....:     cdef long i
....:     for i from 0<=i<m:
....:         try:
....:             raise AttributeError, s%(self.__class__,name)
....:         except AttributeError:
....:             pass
....: """)

That string is a good approximation to what we really want, but the differences to the original error messages could only be resolved by a case distinction:

sage: s = LazyFormat("%s has no attribute %s")
sage: raise AttributeError, s%(QQ.__class__,'bla')
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)

/home/king/<ipython console> in <module>()

AttributeError: <class 'sage.rings.rational_field.RationalField_with_category'> has no attribute bla
sage: QQ.bla
...
AttributeError: 'RationalField_with_category' object has no attribute 'bla'

However, even if we do not insist on reproducing the exact same error message, lazy format strings are simply too slow.

sage: %time test(QQ,'bla',10^6)
CPU times: user 16.12 s, sys: 0.01 s, total: 16.13 s
Wall time: 16.12 s

So, it is nearly eight times slower than using raise_attribute_error in unpatched sage, and about fifty times slower than using AttributeErrorMessage, which was only 3 seconds for 10^7 (not 10^6) iterations, and which does preserve the old error messages.

[hivert]

comment:6

So, it is nearly eight times slower than using raise_attribute_error in unpatched sage, and about fifty times slower than using AttributeErrorMessage, which was only 3 seconds for 10^7 (not 10^6) iterations, and which does preserve the old error messages.

As I already said: [It] is pure Python so that probably a little Cythonizing is needed if it fits you needs.

Plus compared to your code I have have to pay the extra cost of calling the operator __mod__ for % when binding the string to its extra args. If you want speed from cython, it would be better to have this extra binding in a non standard def method so that you can bypass the call to python interpreter. Do you accept a little slow down (calling %) as a trade-off to avoid code and feature duplication ?

[hivert]

comment:7

Hi Simon

Some more info: According to prun most of the time is spend during the copy in

    def __mod__(self, args):
        """
        """
        if hasattr(self, "_args"): # self is already bound...
            self = copy(self)
        self._args = args
        return self

For example

        1    2.828    2.828   19.882   19.882 {_home_florent__sage_temp_popcorn_rouba_net_19739_tmp_0_spyx_0.test}
   999999    2.429    0.000   15.759    0.000 copy.py:65(copy)
  1000000    1.088    0.000   17.054    0.000 lazy_format.py:82(__mod__)
  1999999    1.067    0.000    1.067    0.000 {hasattr}

I'm pretty sure that cythonizing properly this copy should give a large
speedup. So the question is: should we try to optimize LazyFormat or do
you rather have a hand tuned err-message to ensure proper backward
compatibility ?

[simon-king-jena]

comment:8

Replying to @hivert:

As I already said: [It] is pure Python so that probably a little Cythonizing is needed if it fits you needs.

Plus compared to your code I have have to pay the extra cost of calling the operator __mod__ for % when binding the string to its extra args. If you want speed from cython, it would be better to have this extra binding in a non standard def method so that you can bypass the call to python interpreter. Do you accept a little slow down (calling %) as a trade-off to avoid code and feature duplication ?

I don't think that there is any code or feature duplication.

Certainly there is no code duplication.

What you can do with AttributeErrorMessage can not directly be done with a format string (lazy or not), because you need to distinguish two cases. And what you can do with lazy format strings can not be done with an AttributeErrorMessage, since the only thing that the new class can do is to print itself as an error message. So, there is no feature duplication either.

Moreover, this ticket explicitly is about speed. A slow down by an extra call to __mod__ would thus not be acceptable, IMO. In particular if that call makes the attribute access slower than with unpatched sage.

[simon-king-jena]

comment:9

Replying to @hivert:

Some more info: According to prun most of the time is spend during the copy in
[mod].

...
I'm pretty sure that cythonizing properly this copy should give a large
speedup. So the question is: should we try to optimize LazyFormat or do
you rather have a hand tuned err-message to ensure proper backward
compatibility ?

Backward compatibility is one thing.

In addition I don't think that you can come up to speed, even with cython, if you need to copy strings etc. The point of AttributeErrorMessage is to assign two attributes (without copying, so, it just sets two pointers to existing objects) and nothing else.

Of course, you may try to tune the lazy format strings so that they are fast enough for the application here. But for the moment, I really don't see why one should not introduce a very slim new class (it only has __init__ and __repr__), if it is faster and offers backward compatibility for free.

[hivert]

comment:10

In addition I don't think that you can come up to speed, even with cython,
if you need to copy strings etc. The point of AttributeErrorMessage is to
assign two attributes (without copying, so, it just sets two pointers to
existing objects) and nothing else.

Please have a look at the code... I'm just copying the object itself if it is
bound with two different objects. Now using copy.copy from python is always
awfully slow. So I'm basically creating a new object as you have to do.

Of course, you may try to tune the lazy format strings so that they are fast
enough for the application here. But for the moment, I really don't see why
one should not introduce a very slim new class (it only has __init__ and
__repr__), if it is faster and offers backward compatibility for free.

If you tell me you won't use it, I'm less motivated to tune it finely ;-)

[simon-king-jena]

comment:12

Ping!

For the patchbot (since it doesn't read the ticket description):

Apply trac11342-AttributeErrorMessage.patch

[simon-king-jena]

comment:13

On June 27, the patchbot applied both patches, even though on June 26 I pointed out that only one patch has to be applied.

Let's ty again...

Apply trac11342-AttributeErrorMessage.patch

[robertwb]

comment:14

apply only trac11342-AttributeErrorMessage.patch

[robertwb]

comment:15

Stupid auto-wikification apply only trac11342-AttributeErrorMessage.patch

[robertwb]

apply only this patch

[simon-king-jena]

comment:16

Attachment: trac11342-attribute_error_message.patch.gz

Replying to @robertwb:

Stupid auto-wikification

Thank you for your effort and for renaming the patch!

Now, the patchbot has still trouble, namely when applying #9944. That ticket was merged in sage-4.7.1.alpha2, but apparently it can not be applied to sage-4.7.

Hélas.

For the record: It should work on top of sage-4.7.1.rc2.

[simon-king-jena]

Only apply this patch. Make attribute access faster on elements and parents: Create the error message only when needed

[simon-king-jena]

comment:17

Attachment: trac11342-attribute_error_message.rebased.patch.gz

I had to rebase the patch, since it only applied with fuzz 1 against sage-4.7.1.rc1.

Still needs review.

Apply trac11342-attribute_error_message.rebased.patch

[simon-king-jena]

comment:46

Replying to @vbraun:

Ok I looked at sage/rings/pari_ring.py and it makes my eyes bleed... The elements don't call RingElement.__init__ so their parent is not set. Also, should use UniqueRepresentation. But is anybody using that module?

I guess, implicitly we are using it all: As we have seen, some pari objects are generated when sage starts. And you are right, it is evident that sage/rings/pari_ring.py has been written long time ago...

Do you think we should attempt to bring it up to date here? Or do it on a different ticket and just use the work-around provided by attachment: trac_11342_fix_pari_initialization.patch?

If we fix it here: Did you already start with it? Otherwise, I'd do it myself.

[simon-king-jena]

comment:47

Volker, I am not getting the segfault you reported.

With the current two patches, I obtain:

sage: R = PariRing()
sage: loads(R.dumps()) == R
True
sage: loads(R.dumps()) is R  # I want to fix that!
False
sage: f = R('x^3 + 1/2')
sage: f.dumps()
'x\x9ck`J.NLO\xd5+\xca\xccK/\xd6+H,\xca\x8c\x071\xb9\x02\x80,\xaeBF\xcd\xc6B&\xbf\xdaB\xe6P\x8ex\x90H||E!\x0bDCNf\x12D\xbd^zj^|A%W\x01X\x07k(gE\x9c\xb1\x82\xb6\x82\xa1\xbeQkP![q[\x92\x1e\x00\xbc9 \xf7'

So, the question is whether we should fix it here, or open a new ticket and make it a dependency.

[simon-king-jena]

Some fixes for the Pari pseudoring

[simon-king-jena]

comment:48

Attachment: trac_11342_fix_pari_ring.patch.gz

I have already produced a patch on the pari ring problem. Volker, does it make your segfault vanish? At least, I think that my patch improves the code quality of pari_ring.py.

[vbraun]

Attachment: trac_11342_crypto_fixes.patch.gz

Initial patch

[vbraun]

comment:49

Your patch fixes the pari_ring segfaults and looks great!

I've fixed the crash in sage/crypto/classical_cipher.py in the patch that I just posted.

The next crash, in sage/algebras/quatalg/quaternion_algebra.py, is more tricky. Ideals in quaternion algebras don't have their parent set. The QuaternionFractionalIdeal_rational class derives from sage.ring.ideal which assumes it is an ideal in a commutative ring, see sage.ring.ideal_monoid.

[simon-king-jena]

comment:50

Replying to @vbraun:

Your patch fixes the pari_ring segfaults and looks great!

Thank you!

I've fixed the crash in sage/crypto/classical_cipher.py in the patch that I just posted.

OK, I'll have a look (but the problem is that I can't verify that it fixes a segfault, because I don't get a segfault.

The next crash, in sage/algebras/quatalg/quaternion_algebra.py, is more tricky. Ideals in quaternion algebras don't have their parent set. The QuaternionFractionalIdeal_rational class derives from sage.ring.ideal which assumes it is an ideal in a commutative ring, see sage.ring.ideal_monoid.

Too bad. There is #11068, which introduces proper support for one- and twosided ideals in non-commutative rings. But that comes much later in my patch chain...

Perhaps we can use a temporary workaround: __getattr__ could not only test whether self._parent._category is None, but whether self._parent is None.

Of course, any additional test will affect the performance. But perhaps it is still acceptable? I think that would be the easiest solution, so, let's do some tests with it.

[simon-king-jena]

comment:51

The crypto fix looks nice. I didn't run the doc tests yet, but "what could possibly go wrong"?

I think it should be used even if the segfaults could be used in a different way. That's to say, I will add if self._parent is not None to __getattr__ and run all tests with both your new and my to-be-created patch.

[simon-king-jena]

comment:52

I have added a new patch. Since any element has a parent, I do actually not test whether self._parent is None. Instead, I do cdef Parent P = self._parent or self.parent().

That works, because self.parent() is supposed to return something (even in cases of improper elements as we have met them in Pari). Moreover, it is fast, since the slow call self.parent() will not be used if self._parent is not None.

Since the parent needs to be retrieved anyway, my new patch essentially preserves the performance. So, the additional test is essentially for free.

I suppose that my patch would suffice to fix the problem. However, Volker's crypto patch improves the code quality and thus should be included as well.

For me, the tests pass when adding Volker's and my patch. But, after all, I did not suffer from the segfault. So, please see if it's fixed!

Apply trac11342-attribute_error_message.rebased.patch, trac_11342_fix_pari_initialization.patch, trac_11342_fix_pari_ring.patch, trac_11342_crypto_fixes.patch, trac11342_test_parent_on_getattr.patch

[vbraun]

comment:53

Thats not good enough because e.g. the ideals in the quaternion algebra return None as parent(), too. Without this patch:

sage: R = QuaternionAlgebra(-11,-1).maximal_order()
sage: R.right_ideal(R.basis())
Fractional ideal (1/2 + 1/2*j, 1/2*i + 1/2*k, j, k)
sage: R.unit_ideal().parent() is None
True

and with this ticket it still segfaults.

[simon-king-jena]

comment:54

Replying to @vbraun:

Thats not good enough because e.g. the ideals in the quaternion algebra return None as parent(), too.

Too bad. Then I'll try the more radical solution and test whether P is None.

[simon-king-jena]

comment:55

And, by the way, even my patch on ideals in non-commutative rings wouldn't solve the problems with Quaternion algebra, since #11068 is about ideals, not fractional ideals.

[simon-king-jena]

Attachment: trac11342_test_parent_on_getattr.patch.gz

Test whether self._parent is None, before requestin self._parent._category

[simon-king-jena]

comment:56

I just updated my last patch. It should be absolutely airtight, since it is now explicitly tested whether the parent is None.

The timings, which is the main point of this ticket, are still fine. They did almost not change compared with the old patches, but they are much better than with unpatched sage-4.7.2.alpha2:

1. non-existing private attributes

sage: a = 1
sage: timeit('try: QQ.__bla\nexcept: pass')
625 loops, best of 3: 10.8 µs per loop
#unpatched: 15 µs per loop
sage: timeit('try: a.__bla\nexcept: pass')
625 loops, best of 3: 9.07 µs per loop
#unpatched: 13.9 µs per loop
sage: timeit('try: QQ.__bla_\nexcept: pass')
625 loops, best of 3: 14.3 µs per loop
#unpatched: 22.8 µs per loop

2. non-existing non-private attributes

sage: timeit('try: QQ.__bla_\nexcept: pass')
625 loops, best of 3: 14.2 µs per loop
#unpatched: 23 µs per loop
sage: timeit('try: a.__bla_\nexcept: pass')
625 loops, best of 3: 17.3 µs per loop
#unpatched: 23.2 µs per loop
sage: timeit('try: QQ.bla\nexcept: pass')
625 loops, best of 3: 13.9 µs per loop
#unpatched: 22.6 µs per loop
sage: timeit('try: a.bla\nexcept: pass')
625 loops, best of 3: 16.6 µs per loop
#unpatched: 25.9 µs per loop

3. existing attributes inherited from the category

sage: timeit('try: QQ.sum\nexcept: pass',number=10^5)
100000 loops, best of 3: 744 ns per loop
#unpatched: 754 ns per loop
sage: timeit('try: a.cartesian_product\nexcept: pass',number=10^5)
100000 loops, best of 3: 1.68 µs per loop
#unpatched: 3.97 µs per loop

I hope that the segfaults are finally gone!!

Apply trac11342-attribute_error_message.rebased.patch, trac_11342_fix_pari_initialization.patch, trac_11342_fix_pari_ring.patch, trac_11342_crypto_fixes.patch, trac11342_test_parent_on_getattr.patch

[simon-king-jena]

comment:57

When combining this ticket with #9138 then another pari test fails (because #9138 initialises some category that here is expected to be not initialised).

How to proceed? Both tickets are not positively reviewed yet.

[vbraun]

Changed reviewer from Jeroen Demeyer to Jeroen Demeyer, Volker Braun

[vbraun]

comment:58

We should probably merge this ticket first since it is more low-level. I'm happy with it, so positive review for everything except trac_11342_crypto_fixes.patch and trac_11342_fix_pari_initialization.patch (which I wrote). Simon and Jeroen already reviewed those somewhat trivial patches, so everything is good to go.

[simon-king-jena]

comment:59

If it is needed (one could argue that your patches are small enough to be reviewer patches), I give your patches a positive review as well.

[nexttime]

Changed reviewer from Jeroen Demeyer, Volker Braun to Jeroen Demeyer, Volker Braun, Simon King

[nexttime]

Changed author from Simon King to Simon King, Volker Braun

[nexttime]

Merged: sage-4.7.2.alpha3