-
Notifications
You must be signed in to change notification settings - Fork 136
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
PolyRatFun performance regression #178
Comments
For me, the commits since 5634f18 break some code which uses polyratfun. I get
I am at a conference so do not have time to bisect the problem for a few days or to produce a minimal example, but I will get back to you... Josh. |
Hi Josh,
In this case the date of the sources is rather important. We had a similar problem two or three
days ago, but that has been repaired to my knowledge.
We tried to repair some internal inefficiencies in the application of polyratfun.
This involved using some already existing routines, but it turned out they were not exactly
doing what we thought they should be doing (they are part of the original rational polynomial
code by Jan). It turned out a bit more complicated than thought, but we did not want to
go back to the older slower code, because in some cases the regression was enormous.
If you have a crashing example with the latest commit, I would like to get that.
What happened had (in our case) to do with PolyRatFun rat,RAT; and the use of the RAT
function. In the worst case we also have an elegant workaround for that.
Why did we push? It all worked on my examples and nontrivial test runs.
It also passed all the tests in the make check. Then we got the crash eventually in an example
of Takahiro. This was repaired on monday (3 april). Since then I have been trying to tune the
performance a bit more.
This should explain what happened.
Jos
… On 5 apr. 2017, at 00:51, Josh Davies ***@***.***> wrote:
For me, the commits since 5634f18 <5634f18> break some code which uses polyratfun. I get
Division by zero during normalization
I am at a conference so do not have time to bisect the problem for a few days or to produce a minimal example, but I will get back to you...
Josh.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub <#178 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFLxEio6TM2EUXXYjnU0ACAVlSLtwzbPks5rssljgaJpZM4Mug5A>.
|
OK, I managed to produce a minimal example which demonstrates the crash,
This works OK with 5634f18 but gives the division by zero error with 5b55fdb . Interestingly, if I comment the
with the bad commit. Thanks, EDIT: this is introduced by a0b635c |
Hi Josh,
Thanks! That helped.
I found the problem and put the repair in the github.
Cheers
Jos
… On 5 apr. 2017, at 16:25, Josh Davies ***@***.***> wrote:
OK, I managed to produce a minimal example which demonstrates the crash,
#-
Off Statistics;
Symbol a,b,c,ep;
CFunction redprf,epprf;
Local test1 =
+ epprf(-1, - 1 + ep)*redprf(1,1)
+ epprf(-1,1 - 3*ep + 2*ep^2)*redprf(-1,1)
;
.sort
PolyRatFun redprf;
Identify redprf(a?,b?) = redprf(a*c,b*c);
Identify epprf(a?,b?) = redprf(a,b);
.sort
Print +s;
.end
This works OK with 5634f18 <5634f18> but gives the division by zero error with 5b55fdb <5b55fdb> .
Interestingly, if I comment the .sort after the Identify statements, it prints
test1 =
+ redprf( - 2 + 4*ep - 2*ep^2,0)
;
with the bad commit.
Thanks,
Josh.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#178 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFLxEiLYdrWlgAhmHS1pQSqwi3j0XJfRks5rs6RVgaJpZM4Mug5A>.
|
Perhaps now we can turn off this performance regression alert. |
We have observed a considerable performance regression on
PolyRatFun
computations, introduced by ffabb37. For now, difficulty of fixing this regression is unclear.The text was updated successfully, but these errors were encountered: