-
-
Notifications
You must be signed in to change notification settings - Fork 482
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pynac comparison functions do not provide a SWO #9880
Comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The bug happened because of the comparison functions which are used in a call to std::sort. I have finally looked at the comparison functions and exchanging :
by
in mul::compare_pow (mul.cpp:1265) seems to prevent the above bug from happening. It seems to fit better with the change made by William Stein in power::compare_same_type (power.cpp:951). However it doesn't mean the problem is completely solved... I'll try to take a deeper look at the comparison functions at some point. I tested the above fix with pynac 0.2.1. |
Attachment: trac9880_pynac_order_burcin_original.patch.gz Burcin original patch |
Patch to apply on top of the other one |
comment:3
Attachment: trac9880-pynac_order_jp_new.patch.gz We've been working on a patch to fix the issue. Original discussion is here: Burcin produced a patch restoring GiNaC original order for internal use and using the new ones only for printing ; thus fixing the bug. I then worked on top of it to get a more consistent order. You can test them using pynac 0.2.1 from #9901 ("cd spkg/standard/pynac-0.2.1/src/" then "hg import" both patches and build/install, you should "./sage -b" if upgrading from a previous version of pynac). I don't consider that version as definitve, but would like to get some feedback on the order used for printing. I don't use everything pynac provides so it is more than probable that some expression are not correctly printed now. Do not hesitate to report it. |
comment:5
#10282 almost certainly is the same thing. |
comment:6
Fixing this probably will close #9632 - just putting that here for the record. |
comment:7
Apply |
comment:8
(Just OS X, not any particular architecture, I think.) |
Attachment: trac_9880_revert_marking_random_from_trac_10187.patch.gz Re-enable doctests that fail on PPC due to this issue (updated) |
comment:9
I'd like to test this at some point, but have two problems.
|
comment:10
I guess there are two main parts in this ticket, I did not have a look for a long time, so all this should be taken carefully :
|
comment:11
I could review the c++ but the code in the ticket seems to work for me. If it segfaults for you, how about including a full sage session with a stack backtrace at the very least? |
comment:12
There is all you are asking for in the original discussion linked in the ticket description. Maybe something changed since then, the ticket is quite old. |
comment:13
As far as I'm concerned, it still segfaults with the Sage 4.6 package for 32-bit Ubuntu provided on sagemath.org, as well as on a 64 bit Sage 4.6 that I built myself. I did not test it any 4.6.1 alpha or rc yet. |
comment:14
I finally got Sage 4.6.1 final built from scratch, and:
By the way, the problem of different ordering on different architecture should still be present (10187#!comment:39). |
comment:15
Ok, here is a kind of strange example for the problems of ordering still hapenning with Sage 4.6.1:
The expression for f should get (a little bit) simplified. For example, there are different summands where the only symbolic expressions used are (2!b_2)!-2 and they should get automatically gathered when pynac creates the object. In fact calling expand() method on f gives you the right expression, but if things were working correctly you should not have to do this. |
comment:16
Can someone experiencing this bug please provide a gdb backtrace? There is a (small) possibility that this issue here might be related to an issue I encounter with sage on Gentoo Linux (with segfault in PyNaC as well), and I'd like to know for sure whether the error occurs inside the function |
comment:17
The detailed description of the segfault mentionned here is available in the discussion mentionned in the ticket description. I'll put it here so that everybody finds it:
!#0 GiNaC::power::compare (this=0x449be20, other=...) at !power.cpp:899 And valgrind output also: |
comment:18
Replying to @jpflori:
I take it that Is the patch in this ticket still relevant or has this been fixed elsewhere? |
comment:19
The problem is not the GiNaC original internal order (even if http://www.ginac.de/reference/structGiNaC_1_1expair__rest__is__less.html states it is not a SWO... at least using it makes the bug disappear and inconsistencies in automatic simplifications disappear; if it does not work, i guess it should be fixed by GiNaC team) but the modification of that order used in pynac (and so in Sage). AFAIK the two patches here are still necessary, the segfault disappeared (!?!) but i still get some problems with automatic simplification as shown in #9880 comment:15.
It should not be much more difficult to implement other orders for printing (and operands access). |
comment:20
I have a suspicion that gcc's |
comment:21
I don't understand the GiNaC documentation that you referred to (http://www.ginac.de/reference/structGiNaC_1_1expair__rest__is__less.html). They state it is not a SWO, and their example is that neither 3x<2x nor 2x<3x. But thats perfectly fine in a SWO, you can have incomparable elements. The only constraint on incomparable elements is transitivity, that is, if A and B are incomparable and B and C are incomparable then A and C are also incomparable. Do you understand why its not a SWO? |
comment:91
Please rebase to #14550. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
comment:94
Please rebase to #9890 (the latest patch applies with fuzz 2). |
This comment has been minimized.
This comment has been minimized.
comment:95
Attachment: trac_9880-fix_doctests-sage_5_10_beta2.take3.patch.gz Replying to @jdemeyer:
Done. |
Merged: sage-5.11.beta0 |
Here is a short example found by Burcin and reproducing the bug:
This is already being discussed here: http://groups.google.com/group/sage-support/browse_thread/thread/7c85f02c76012722
The following patches are for the Sage library to enable access to the PyNaC order and randomly test that it is a SWO:
Install the package from here: http://boxen.math.washington.edu/home/jpflori/spkg/pynac-0.3.0.spkg
Then apply
Depends on #13213
Depends on #9890
Depends on #14550
CC: @kcrisman @zimmermann6
Component: symbolics
Keywords: pynac spkg
Author: Burcin Erocal, Jean-Pierre Flori, Volker Braun
Reviewer: Burcin Erocal, Jean-Pierre Flori
Merged: sage-5.11.beta0
Issue created by migration from https://trac.sagemath.org/ticket/9880
The text was updated successfully, but these errors were encountered: