-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Implement matrix truncation for latex to enhance the display of large matrices #26526
base: master
Are you sure you want to change the base?
Conversation
…ting The `_print_matrix_contents` method now includes options for compactifying and setting maximum visible rows and columns, providing a more readable representation. This improvement prevents printing slowdowns for large matrices and improves overall user experience. This modification shall solve sympy#16251.
✅ Hi, I am the SymPy bot. I'm here to help you write a release notes entry. Please read the guide on how to write release notes. Your release notes are in good order. Here is what the release notes will look like:
This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.13. Click here to see the pull request description that was parsed.
|
Benchmark results from GitHub Actions Lower numbers are good, higher numbers are bad. A ratio less than 1 Significantly changed benchmark results (PR vs master) Significantly changed benchmark results (master vs previous release) | Change | Before [2487dbb5] | After [bd0b3e6f] | Ratio | Benchmark (Parameter) |
|----------|----------------------|---------------------|---------|----------------------------------------------------------------------|
| - | 67.2±1ms | 43.0±0.2ms | 0.64 | integrate.TimeIntegrationRisch02.time_doit_risch(10) |
| + | 18.4±0.2μs | 30.2±0.04μs | 1.64 | integrate.TimeIntegrationRisch03.time_doit(1) |
| - | 5.36±0.03ms | 2.89±0.05ms | 0.54 | logic.LogicSuite.time_load_file |
| - | 73.3±0.3ms | 29.0±0.2ms | 0.4 | polys.TimeGCD_GaussInt.time_op(1, 'dense') |
| - | 25.7±0.1ms | 16.9±0.03ms | 0.66 | polys.TimeGCD_GaussInt.time_op(1, 'expr') |
| - | 73.2±0.6ms | 29.5±0.1ms | 0.4 | polys.TimeGCD_GaussInt.time_op(1, 'sparse') |
| - | 260±2ms | 125±0.4ms | 0.48 | polys.TimeGCD_GaussInt.time_op(2, 'dense') |
| - | 258±1ms | 126±0.7ms | 0.49 | polys.TimeGCD_GaussInt.time_op(2, 'sparse') |
| - | 657±5ms | 377±3ms | 0.57 | polys.TimeGCD_GaussInt.time_op(3, 'dense') |
| - | 662±4ms | 374±3ms | 0.57 | polys.TimeGCD_GaussInt.time_op(3, 'sparse') |
| - | 497±5μs | 291±3μs | 0.58 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(1, 'dense') |
| - | 1.81±0.02ms | 1.06±0ms | 0.58 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(2, 'dense') |
| - | 5.77±0.04ms | 3.08±0.01ms | 0.53 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(3, 'dense') |
| - | 452±2μs | 231±2μs | 0.51 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(1, 'dense') |
| - | 1.48±0ms | 696±10μs | 0.47 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(2, 'dense') |
| - | 4.92±0.02ms | 1.67±0.01ms | 0.34 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(3, 'dense') |
| - | 374±1μs | 206±1μs | 0.55 | polys.TimeGCD_SparseGCDHighDegree.time_op(1, 'dense') |
| - | 2.41±0.01ms | 1.25±0.01ms | 0.52 | polys.TimeGCD_SparseGCDHighDegree.time_op(3, 'dense') |
| - | 9.97±0.09ms | 4.45±0.05ms | 0.45 | polys.TimeGCD_SparseGCDHighDegree.time_op(5, 'dense') |
| - | 357±2μs | 169±0.9μs | 0.47 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(1, 'dense') |
| - | 2.49±0.03ms | 917±4μs | 0.37 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(3, 'dense') |
| - | 9.47±0.1ms | 2.64±0.01ms | 0.28 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(5, 'dense') |
| - | 1.02±0.01ms | 431±4μs | 0.42 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'dense') |
| - | 1.72±0.02ms | 501±3μs | 0.29 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'sparse') |
| - | 5.80±0.04ms | 1.83±0.01ms | 0.32 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'dense') |
| - | 8.46±0.05ms | 1.48±0ms | 0.18 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'sparse') |
| - | 286±2μs | 65.4±0.2μs | 0.23 | polys.TimePREM_QuadraticNonMonicGCD.time_op(1, 'sparse') |
| - | 3.43±0.03ms | 396±3μs | 0.12 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'dense') |
| - | 3.96±0.02ms | 275±1μs | 0.07 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'sparse') |
| - | 6.98±0.07ms | 1.29±0.01ms | 0.18 | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'dense') |
| - | 8.71±0.07ms | 834±10μs | 0.1 | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'sparse') |
| - | 5.08±0.04ms | 3.32±0.01ms | 0.65 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(2, 'sparse') |
| - | 12.0±0.04ms | 6.69±0.01ms | 0.56 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'dense') |
| - | 22.8±0.2ms | 9.03±0.02ms | 0.4 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'sparse') |
| - | 5.49±0.01ms | 870±5μs | 0.16 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(1, 'sparse') |
| - | 12.6±0.03ms | 6.98±0.02ms | 0.55 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(2, 'sparse') |
| - | 101±0.7ms | 26.0±0.1ms | 0.26 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'dense') |
| - | 173±2ms | 54.0±0.2ms | 0.31 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'sparse') |
| - | 175±0.7μs | 114±0.4μs | 0.65 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(1, 'dense') |
| - | 368±1μs | 213±2μs | 0.58 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(1, 'sparse') |
| - | 4.25±0.03ms | 846±7μs | 0.2 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'dense') |
| - | 5.24±0.04ms | 379±2μs | 0.07 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'sparse') |
| - | 19.8±0.2ms | 2.83±0.01ms | 0.14 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'dense') |
| - | 22.8±0.09ms | 624±1μs | 0.03 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'sparse') |
| - | 498±2μs | 136±1μs | 0.27 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(1, 'sparse') |
| - | 4.69±0.03ms | 623±2μs | 0.13 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'dense') |
| - | 5.28±0.02ms | 137±1μs | 0.03 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'sparse') |
| - | 13.1±0.2ms | 1.28±0ms | 0.1 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'dense') |
| - | 13.8±0.07ms | 140±1μs | 0.01 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'sparse') |
| - | 132±1μs | 73.5±0.3μs | 0.56 | solve.TimeMatrixOperations.time_rref(3, 0) |
| - | 252±5μs | 87.1±0.3μs | 0.35 | solve.TimeMatrixOperations.time_rref(4, 0) |
| - | 24.0±0.3ms | 10.2±0.02ms | 0.43 | solve.TimeSolveLinSys189x49.time_solve_lin_sys |
| - | 29.1±0.1ms | 15.5±0.04ms | 0.53 | solve.TimeSparseSystem.time_linsolve_Aaug(20) |
| - | 54.8±0.1ms | 25.0±0.2ms | 0.46 | solve.TimeSparseSystem.time_linsolve_Aaug(30) |
| - | 28.2±0.1ms | 15.3±0.1ms | 0.54 | solve.TimeSparseSystem.time_linsolve_Ab(20) |
| - | 54.7±0.3ms | 24.5±0.2ms | 0.45 | solve.TimeSparseSystem.time_linsolve_Ab(30) |
Full benchmark results can be found as artifacts in GitHub Actions |
This looks good although I am not sure about the option names. There should be tests for what happens under different settings. Ideally all printers would have this option and in fact displaying large matrices should not be done by default. I wonder about having something like a symbolic |
sympy/printing/latex.py
Outdated
mat_compact: boolean, optional | ||
Truncate large Matrix for printing. Default is ``True``, to avoid printing | ||
slowdown. | ||
mat_visible_rows: int, optional | ||
Sets the maximum number of rows to be printed. Default is ``15``, and | ||
it doesn't take effect if mat_compact is ``False``. | ||
mat_visible_cols: int, optional | ||
Sets the maximum number of columns to be printed. Default is ``15``, and | ||
it doesn't take effect if mat_compact is ``False``. |
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.
I don't think that this many matrix specific options are warranted.
Let's start with something basic: we can have an option like truncate
that defaults to true. It could be used for more than just matrices but for now it could be implemented only for matrices. Ideally other printers like str/repr
should support the same and it should be controllable via init_printing
.
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.
Ok, I just made the changes for latex printing. I agree truncation should also work for the other printers and I will try to get that working. Should I put that in this PR or open a new one?
Also, do you think there is a way to implement this for many of the printers simultaneously(srepr, unicode, etc), or will it have to be a new implementation for each individual printer?
renamed mat_compact to truncate, removed visible_rows and visible_columns parameters. Added tests for each truncatable case.
sympy/printing/latex.py
Outdated
@@ -166,6 +166,7 @@ class LatexPrinter(Printer): | |||
"parenthesize_super": True, | |||
"min": None, | |||
"max": None, | |||
"mat_compact": True, |
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.
I still don't think mat_compact
is the right kind of option to add. Rather there should be a general option for shortening all possible outputs (not just matrices). Its purpose should be to avoid producing absurdly large output in general even if for now it is only applied to matrices.
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.
I agree, just forgot to change mat_compact to truncate in a few spots. The docstring specifies that truncate currently only truncates matrix but might be extended to truncate other things in the future.
should now be.
sympy/printing/latex.py
Outdated
@@ -166,6 +166,7 @@ class LatexPrinter(Printer): | |||
"parenthesize_super": True, | |||
"min": None, | |||
"max": None, | |||
"truncate": True, |
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.
Using truncate=False
expresses what is wanted in a negated sense. Maybe what would be better is the opposite like full_output=True
(default is False
) or something like that...
This naming makes more sense if we plan to add a lot more truncating operations to sympy, beyond just matrices.
References to other Issues or PRs
Fixes #16251.
Brief description of what is fixed or changed
I fixed the merge conflict for PR#26284 which seems to have been abandoned. It should auto-merge into master now.
Other comments
Original Author: mohamedrezk122
Release Notes