Skip to content
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

Fix usage of symbolic comparison in several places #24992

Open
rwst opened this issue Mar 16, 2018 · 9 comments
Open

Fix usage of symbolic comparison in several places #24992

rwst opened this issue Mar 16, 2018 · 9 comments

Comments

@rwst
Copy link

rwst commented Mar 16, 2018

Symbolic comparison is wrongly used in several places throughout Sage (overview https://trac.sagemath.org/wiki/symbolics/nonzero). The ticket replaces code involuntarily calling __nonzero__ with a call of the is_zero or is_equal member functions to clarify if simplification is needed or not. The code positions calling __nonzero__ were obtained by raising an exception in __nonzero__ and analyzing the resulting doctest fails. Of course not every simplification is unwanted so every case must be considered and tested.

CC: @cheuberg @dkrenn

Component: performance

Author: Ralf Stephan

Branch/Commit: u/rws/fix_usage_of_symbolic_comparison_in_several_places @ d08957e

Issue created by migration from https://trac.sagemath.org/ticket/24992

@rwst rwst added this to the sage-8.2 milestone Mar 16, 2018
@rwst

This comment has been minimized.

@rwst

This comment has been minimized.

@rwst
Copy link
Author

rwst commented Mar 17, 2018

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 18, 2018

Branch pushed to git repo; I updated commit sha1. New commits:

e1942d024992: fixes

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 18, 2018

Commit: e1942d0

@rwst
Copy link
Author

rwst commented Mar 18, 2018

comment:5

I get a 10-15% decrease of CPU time with this branch when doing sage -tp src/sage/rings/asymptotic/ (from 31.5s to 26.5-28.5s).

It's not finished, there are more instances in Sage where the changes can be applied.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 19, 2018

Changed commit from e1942d0 to d08957e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 19, 2018

Branch pushed to git repo; I updated commit sha1. New commits:

d08957e24992: more optimizations

@rwst
Copy link
Author

rwst commented Mar 19, 2018

Author: Ralf Stephan

@mkoeppe mkoeppe modified the milestones: sage-8.2, sage-9.4 Jul 6, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Aug 22, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 2, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants