Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 matrix_exp_multiply accuracy bug #2619
fix matrix_exp_multiply accuracy bug #2619
Changes from 17 commits
381ad68
25a7d8b
97fc786
c15984a
564757e
4b78bf6
fb56049
49dc3e5
bf2e745
f708734
1081352
ea55ec7
fac3d0c
190a2c0
f0f462f
a53874a
deccced
082f7b6
8d9bbad
4466988
d6777b0
d018c4c
9ea04de
079c66e
74df8a5
e40da15
35c259b
f9476a1
df93668
8feefd4
42228ef
f4b6c42
8f1c923
f46628b
3e12a84
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 almost want to split the condition. If
t < tol
, then we don't have to take a norm.I might do that.
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'd want to eagerly return here so it's clear that there's no more that happens below it.
That will unnest the lower
else
statement and it'll be slightly easier to read.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 would put whitespace above 185 and remove the whitespace in 186.
The output of the
if
statement here isalpha
. We want to keep that declaration close together if possible for readability.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.
for efficiency, I'd move the
/ (m_max * b.cols())
to the left hand side by* (m_max * b.cols())
.