-
Notifications
You must be signed in to change notification settings - Fork 139
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
gcd_ doesn't give the correct result #260
Comments
Another (shorter) example:
|
This must be a bug in the C++ code, because |
Yes, it is. I'll go after it. Some preliminary info:
It's been a while since I looked at this code, but the numbers in the heuristics function don't seem quite right. However, the real wrong result is caused by the |
The correct result is obtained if the univariate content is not removed from the result at the end of Edit: we should likely only divide by the integer content, as described in Algorithms for Computer Algebra by Geddes. In the paper Computing the Greatest Common Divisor of |
@benruijl Thanks. Your commit will fail to pass the tests on Travis CI, but this is due to some upgrade (against Meltdown) on the Travis side. I will fix it. |
Should we suppress "New GCD attempt (unused vars)..." message? |
Yes, that should be removed. Sorry!
Op di 23 jan. 2018 18:42 schreef Takahiro Ueda <[email protected]>:
… Should we suppress "New GCD attempt (unused vars)..." message?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#260 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AARGGfVTBX9UeAZjfreJOSkrXEFi3DRdks5tNhnwgaJpZM4RorU3>
.
|
Do you want to do that? Or I can do with adding test cases. |
You can do it :)
Op di 23 jan. 2018 om 18:57 schreef Takahiro Ueda <[email protected]
…:
Do you want to do that? Or I can do with adding test cases.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#260 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AARGGcrFniACflZMIMHZzWq09EOvq0Iwks5tNh2SgaJpZM4RorU3>
.
|
This fix surely affects on PolyRatFun with many symbols:
In the above case, the reduction of the fraction was not perfect with the previous version. |
Thanks to this fix, now I can make some benchmark plot for GCD of random polynomials, inspired by plots of Rings. In case you are interested, the slowest gcd (the right-most point; FORM: 140.5s, Mathematica: 3.3s) is here. |
Hey @tueda, this benchmarking script is really interesting. I added support for Fermat which is frequently used for its fast GCD in integral reduction codes (e.g. Reduze and Fire). You can find the extended script here. Surprisingly the FORM GCD performs much better than Fermat on relatively simple polynomials (the default settings of your script). Perhaps this indicates that I implemented the link to Fermat poorly? However, as I increase the complexity (especially the number of terms in the polynomial) Fermat becomes much more competitive and soon greatly outperforms FORM for the types of examples we typically encounter during our reduction. I added to the README some example parameters that more closely mirror what we encounter in our reduction. |
Actually the updated script now has options to run Fermat and Rings, via libFermat and Ammonite REPL, respectively :-) I got results that FORM wins against them by the default benchmark parameters, but I didn't upload the plots because these may be totally dependent on the specific examples (and I'm not sure about the license for Fermat at Nikhef). libFermat is written in C++ and just uses stdin and stdout (redirected by pipes), so it is almost what we can do best, I think, and still I got a bit disappointed result. In this sense, the slowness of Fermat for the default benchmark parameters via pipes in your case is not due to your Python implementation (to some extent; certainly running Fermat for each problem has some overhead. I run Fermat once for the whole problem set). On the other hand, it is claimed that Rings is faster than Mathematica (for their examples), but this is not the case in my default examples. I'm not sure the REPL would have heavy overhead, or just because of difference in examples. Again, these results may totally depend on the benchmark parameters. Even for the default parameters, FORM has some unlucky cases where FORM is somehow very slow (this would be a clue to improve the FORM's GCD performance for more complicated problems). I haven't tried for more complicated inputs with more terms so much, where Fermat (and the other programs) would outperform FORM. |
I just tried
|
Hi @tueda, another code which is used for GCD by Reduze is GiNaC, though its performance is usually very much worse than Fermat (see Fig 3 of the Reduze 2 paper). I wish it was legal/possible for you to make your full testing script public so that there was a single public tool for GCD testing of all interesting packages. One thing that I find odd with your above numbers is that Mathematica is very much faster than Fermat. I wonder: why does FIRE bother to implement a link to Fermat from a mostly Mathematica code if it has a fast GCD available within Mathematica? |
The script also has I'm not sure but when FIRE was firstly programmed maybe the situation was different (the paper for the public FIRE 3.0 appeared in 2008). Or, the benchmark just tests polynomial GCD; performance for rational function arithmetic would be different, possibly. |
Dear @tueda, I just looked your benchmark script -- the part where you call Rings library. You got a dramatically low performance for Rings because of one reason: you run a separate Java instance for each GCD example (via So, to use Rings correctly in your benchmark, you need to have a single instance of I'm not sure, but I suppose you can do something like this in python: # this should be a single instance for all GCD computations
ringsProcess = subprocess.Popen(['rings.repl'], stdin=PIPE, stdout=PIPE, bufsize=1)
# do some GCD example
def solveWithRings(some_data):
# write input
print >>ringsProcess.stdin, "rings_code_for_gcd"
# flush process input
ringsProcess.stdin.flush()
result = ""
for i in range(how_much_lines_you_expect_in_output):
# read result
result = result + ringsProcess.stdout.readline()
return result
# first invocation is quite slow (code is just interpreted)
result_1 = solveWithRings(some_example_1)
# this will be much faster (JIT will compile "hot" methods)
result_2 = solveWithRings(some_example_2)
# this will be even more faster (JIT will compile more "hot" methods)
result_3 = solveWithRings(some_example_3)
# etc... I'll try to incorporate this in your python script later this week, but if you do this faster, I'll really appreciate it since it is very interesting) |
@PoslavskySV, thanks for your comment. Actually I worried that maybe I was doing something wrong. But I use only one for (i <- 1 to N) {
...
} Then this script is put into the standard input of os.system('rings.repl <rings.tmp.prog >/dev/null 2>&1') though I'm not sure if JIT is guaranteed to work in REPL. |
@tueda That does make sense! Sorry, I didn't figure that promptly:) I am worried about the numbers
because that is completely (even extremely!) different from what I had earlier. I'll look closely what is the reason and let you know. Thanks for sharing your script! |
@tueda On my machine (Intel Core i7-7700 CPU @ 4.0 GHz, openSUSE Tumbleweed 20180219, 64 GB RAM) the time that your script reports for Mathematica 11 is not very accurate.
I get:
Measuring this with a wall clock suggests that the "real" time reported is accurate. Can you reproduce this? |
On the same machine for Mathematica 8.0 - 10.4.1 the timing is also inaccurate.
I get:
However, older versions of Mathematica are very much faster:
For comparison:
The results of Mathematica 6.0 and Fermat 6.19 agree. |
I got the following numbers on my Desktop PC:
The setup is less than 1 minute, so, yes there is some gap. This would be attributed to I/O: the number of Actually the FORM benchmark has a disadvantage for this point: it utilizes Your observation that older Mathematica is faster than newer one is very interesting! |
Wait, you got 37m wall clock time for 340.467s of gcds...?? hmm... Apparently I didn't look in your real time numbers carefully, and they have really big gaps. |
Yes, around 30 of 37 minutes was lost with Mathematica v8-11. I find it surprising too. If you can't reproduce this then probably it is a quirk of my machine and it is best to ignore it for this discussion. I will look into it. |
@tueda Takahiro, I also noticed that random polynomials generated with So, to make random polys generated closer to those occur in practice, I recommend to change method as follows: def random_poly(nvars, degree, nterms, coeffpow):
...
while True:
polynomial = ''
for i in range(nterms):
coeff = random_coeff(m)
monomial = ''
sum_degree = degree + random.randint(0, degree)
# if i == 0:
# if flag1 and nvars <= sum_degree:
# for j in range(nvars):
# monomial += '*x{0}'.format(j + 1)
# sum_degree -= nvars
xx = list(range(nvars))
random.shuffle(xx)
for j in xx:
# these 2 lines caused polys to be monic in some variables
# p = random.randint(0, sum_degree)
# sum_degree -= p
p = random.randint(0, sum_degree / nvars)
if p != 0:
monomial += '*x{0}^{1}'.format(j + 1, p)
polynomial += '+({0}){1}'.format(coeff, monomial)
if random.randint(0, 3) == 0:
polynomial += "+1"
form.write('#$a={0};'.format(polynomial))
polynomial = form.read('$a')
if polynomial != '0':
return polynomial With this modification the timings distribution become different. Also, I noticed that FORM hangs in some cases (I digged into FORM's code, and probably the reason is that it misses LinZip algorithm for non-monic case GCD ? but I'm not sure). I tried the following benchmark:
and got the following result:
BTW: I've also found the reason of performance dip in Rings; I already fixed it locally, will release in version 2.3.1 on the next week. |
Thanks. I was trying to make some uneven distribution of powers of variables with some probability, but I didn't know that the GCD algorithm is so sensitive for such cases. Maybe I would put an command line option to select random polynomial generator based on your suggestion. |
@PoslavskySV Could you file a bug report for cases where the gcd computation hangs? The non-monic linzip should be in there, but there is likely an error in the algorithm (I suspect an improper extraction of the content or something like that). |
@benruijl I just re-run the benchmark, waited more time (I used a large |
@benruijl I've added several debugging flags to the script, and found that FORM is constantly slow with
(Rings timings are not very accurate -- they are from my develop, which contains some fixes, but they are still relevant , Mathematica timings are close to Rings on these examples, so I didn't paste them here). All these examples are non-monic case GCD, however in many of them, still the monic-case algorithm with some "scaling corrections" may be applied (as described in [dKMW05]); problems where the use of LinZip is unavoidable (i.e. where Rings switched to LinZip) are marked in bold. I'm not sure whether the reason of slowness is just the non-monic case, since GCD over Z requires some more work to do than plain Zippel/heuristic algorithm. To detect this certainly, it would be useful to check GCD over some (sufficiently large) finite field, say [dKMW05] J de Kleine, M B Monagan, A D Wittkopf. Algorithms for the Non-monic Case of the Sparse Modular GCD Algorithm. Proceeding of ISSAC ‘05, ACM Press, pp. 124-131 , 2005. |
In the following code, FORM is expected to give
gcd(a*g,b*g) = g
,but gives
1
:Maybe related to #258 (if the problem is, for example, in the C wrapper). If the problem is some bug in the C++ code, then rational polynomial arithmetic may also be unreliable.
The text was updated successfully, but these errors were encountered: