-
Notifications
You must be signed in to change notification settings - Fork 15
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
Improve the product of TaylorModels #81
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
@mforets Do you have any comments on this PR? |
I don't have further comments about the implementation, looks good to me 🎉 About the evaluation of the new method, it would be interesting to run the ARCH-NLN RE on |
I don't know for sure but maybe there is a difference only in models handling "small" numbers; in that sense, the Production-Destruction model is a good candidate. |
It seems a good suggestion to me; can you check that, @UzielLinares ? |
I've checked out the suggested for the ProductionDestrucion model and found the following
This new implementation of the product takes 1.17x compared to the implementation on master. |
Though ~20% slower run times is not too bad, the actual improvement of the remainder using the proposed changes is not actually seen. My guess is that the changes may be seen for larger boxes. I propose to leave this PR open, and try to speed up the functions involved, probably in @mforets @dpsanders: Do you have any comments or suggestions? |
thanks for checking !
sounds good to me. i'd also like to play a bit with the new method. we could benchmark |
Another option is to run the benchmark suite in master and this branch, and check how different the results are with respect to time and also the final remainders. But this may be time consuming. |
This PR implements the product of TaylorModel's product as discussed in #80.