Opened 13 years ago. Closed 13 years ago. Implements computations of properties which form double cosets. This algorithm is pretty close to the canonical label algorithm, but it is a more efficient way to implement the isomorphism question. If the objects are not isomorphic, it will tend to discover this pretty quickly, via refinement invariants and examining the partition structure. If they are isomorphic, chances are this isomorphism will be discovered quickly and the algorithm will terminate at that moment.
Download all attachments as:. You need a more current release. My guess is that once you apply to rc3 it will work with that release or even earlier ones.
This seems fairly serious so I'm guessing I should move one to rc4 instead. Sorry for the delay. I tried soing something witth this at work but my machine there is relatively slow. This looks like it was copied from Integer. We also made the change suggested by Robert in Integer. I applied I would also point out that there is no point in declaring 'l' as a 'cdef object' -- that's the cython default. As to the original bug, it seems quite intrinsic to me that digits would be slower than str.
The digits method needs to create individual python objects for each and every digit, str does not. There's a frikkin lot of memory allocation going on there.
With that in mind, I'm not at all convinced that the bug is even fixable at all. Oh, I see, upon more reflection. I see that my idea of "big" wasn't quite big enough. Once, we have a really big number, this patch becomes effective. I do see that my memory allocation comment is entirely incorrect though.
Merging this code is more important than letting it go staler. Thanks for catching the two "long" doctest failures I let slip by. They're indeed trivial to fix.
Please see my "Note 4" from above about the file "boolopt. Currently this ticket already has three different patches to be applied from three different guys. The long doctests now do pass but there is the coverage problem, which is essentially a hen-and-egg problem: the code does not go into Sage because of the coverage, but who would work on code not being in Sage to make the coverage higher?
In my experience, the motivation to increase coverage is the highest when trying to get something in. How essential is boolopt. Could it be pushed to a separate ticket enhancement. Please note that the file "logic. So it couldn't possibly block this ticket to finally go in. But of course, it needs to be cleaned up itself.
The main point here is that added functionality must be doctested while it is nice to increase the coverage on existing code that is being worked on.
So I agree with the approach here. In addition you should open an enhancement ticket to get coverage of logic. So what happens here is one should apply the following in the specified order as suggested by Georg:. Apart from this fuzz, all the above three patches applied OK in the specified order against 3. Doctests including -long passed. In the class BooleanFormula? The same mistake is repeated a few times in the documentation. Minh, thanks for your good work!
After applying the five patches Minh mentioned in the correct order to Sage So I rebased and made it such that Wilfried's name appears in it as originator. Positive review to the sequence of five patches Minh mentioned. And positive review to the changes of Wilfried I really did nothing but rebase them. The main author here is Chris Gorecki, of course. After this ticket is finally closed Requiescat In Pacem , the next steps would be to dismiss "logic. Note that there are some odd whitespace issues in those files, so if someone could take care of that at it would be splendid :.
Powered by Trac 1. Opened 14 years ago Closed 13 years ago. Status badges. Description last modified by mabshoff This is a single commit bundle, so if somebody could extract the patch and post it here it would be a good idea. Cheers, Michael. Attachments 16 logic. Download all attachments as:. How can this ticket be closed when there is no patch here? Is there a patch somewhere else? If so, where?! I am reopening this since David wrongly thought that committing his changes was all that was needed.
He will upload a patch sometime soon. This looks pretty good to me though I have not done any detailed testing, I only looked at the code. I have one suggestion which should speed this up whenever the actual orders of P or Q are strictly less than n: find the orders of P and Q, say m1 and m2.
I also have a question: rather than wait for a division by zero error to catch dependent input, why not do a discrete log calculation to test this? Maybe that's much slower for large n; it would be nice to have this decision justified. Yes, I agree - for now the only thing implemented is for points of order n, I check that they both are of order n, which actually will take some time for large n. There is definitely room for improvement here!
Well, it is much slower for large n to do discrete log computations. I succesfully applied the patch to 3. But the first example I tried crashed:. The comments in the code suggest that 1 run-time errors only happen when the points are independent, and 2 these are caught.
So here is a counterexample to both. Well, I kinda hope that there is, since otherwise it would be a larger problem. Debugging time!
0コメント