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

Revert change to cse #12805

Merged
merged 4 commits into from Jun 27, 2017

Conversation

Projects
None yet
4 participants
@bjodah
Member

bjodah commented Jun 25, 2017

This reverts changes to cse_main from gh-11232 which unfortunately degraded the CSE performance too much (see sympy/sympy_benchmarks#38 & gh-12411).

Timings for test_cse__performance on my laptop:

  • master: 935 s
  • this PR: 0.1 s

EDIT: We should reopen gh-10228, when merging this.

bjodah added some commits Jun 25, 2017

Revert gh-11232
This reverts commit c4c114e.
This reverts commit 97e45da.

@bjodah bjodah added this to the SymPy 1.1 milestone Jun 25, 2017

@isuruf

This comment has been minimized.

Show comment
Hide comment
@isuruf

isuruf Jun 26, 2017

Member

Can you add #12070 as a test here?

Member

isuruf commented Jun 26, 2017

Can you add #12070 as a test here?

@bjodah

This comment has been minimized.

Show comment
Hide comment
@bjodah

bjodah Jun 26, 2017

Member

@isuruf thanks, added it in the form of counting operations, including assignments. (6 vs 11 operations).
Is that what you had in mind?

Member

bjodah commented Jun 26, 2017

@isuruf thanks, added it in the form of counting operations, including assignments. (6 vs 11 operations).
Is that what you had in mind?

@isuruf

This comment has been minimized.

Show comment
Hide comment
@isuruf

isuruf Jun 26, 2017

Member

Thanks. that's perfect.

Member

isuruf commented Jun 26, 2017

Thanks. that's perfect.

@bjodah

This comment has been minimized.

Show comment
Hide comment
@bjodah

bjodah Jun 26, 2017

Member

Thanks @smichr, upon merging this we should re-open that issue.
(was 5df9a53 what you had in mind?)

Member

bjodah commented Jun 26, 2017

Thanks @smichr, upon merging this we should re-open that issue.
(was 5df9a53 what you had in mind?)

@bjodah bjodah changed the title from [WIP] Revert cse to Revert change to cse Jun 26, 2017

@asmeurer

This comment has been minimized.

Show comment
Hide comment
@asmeurer

asmeurer Jun 26, 2017

Member

@smichr can this be merged?

Member

asmeurer commented Jun 26, 2017

@smichr can this be merged?

@smichr

This comment has been minimized.

Show comment
Hide comment
@smichr

smichr Jun 27, 2017

Member

Leave a comment

@smichr can this be merged?

done

Member

smichr commented Jun 27, 2017

Leave a comment

@smichr can this be merged?

done

@smichr smichr merged commit becb102 into sympy:master Jun 27, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@bjodah bjodah deleted the bjodah:revert-cse branch Jun 27, 2017

@bjodah

This comment has been minimized.

Show comment
Hide comment
@bjodah

bjodah Jun 27, 2017

Member

Opened gh-10228

Member

bjodah commented Jun 27, 2017

Opened gh-10228

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment