Avoid using np.linalg.inv() for transform inversion#1418
Avoid using np.linalg.inv() for transform inversion#1418goodboy wants to merge 2 commits intopyqtgraph:masterfrom
np.linalg.inv() for transform inversion#1418Conversation
|
Seeing stuff like this from draw threads: /home/goodboy/repos/pyqtgraph/pyqtgraph/functions.py:2329: RuntimeWarning: invalid value encountered in double_scalars
invA[0, 0] = (-A[1, 2] * A[2, 1] +
/home/goodboy/repos/pyqtgraph/pyqtgraph/functions.py:2331: RuntimeWarning: invalid value encountered in double_scalars
invA[1, 0] = (A[1, 2] * A[2, 0] -
/home/goodboy/repos/pyqtgraph/pyqtgraph/functions.py:2333: RuntimeWarning: invalid value encountered in double_scalars
invA[2, 0] = (-A[1, 1] * A[2, 0] +
/home/goodboy/repos/pyqtgraph/pyqtgraph/functions.py:2335: RuntimeWarning: invalid value encountered in double_scalars
invA[0, 1] = (A[0, 2] * A[2, 1] -
/home/goodboy/repos/pyqtgraph/pyqtgraph/functions.py:2337: RuntimeWarning: divide by zero encountered in double_scalars
invA[1, 1] = (-A[0, 2] * A[2, 0] +
/home/goodboy/repos/pyqtgraph/pyqtgraph/functions.py:2339: RuntimeWarning: invalid value encountered in double_scalars
invA[2, 1] = (A[0, 1] * A[2, 0] -
/home/goodboy/repos/pyqtgraph/pyqtgraph/functions.py:2341: RuntimeWarning: invalid value encountered in double_scalars
invA[0, 2] = (-A[0, 2] * A[1, 1] +
/home/goodboy/repos/pyqtgraph/pyqtgraph/functions.py:2343: RuntimeWarning: invalid value encountered in double_scalars
invA[1, 2] = (A[0, 2] * A[1, 0] -
/home/goodboy/repos/pyqtgraph/pyqtgraph/functions.py:2345: RuntimeWarning: invalid value encountered in double_scalars
invA[2, 2] = (-A[0, 1] * A[1, 0] +So obviously this impl is a bit naive 😸 I can confirm this is drastically improving performance of my app including latency and CPU usage especially with loading larger data sets. |
pyqtgraph/functions.py
Outdated
| @@ -2320,7 +2358,8 @@ def invertQTransform(tr): | |||
| try: | |||
| import numpy.linalg | |||
There was a problem hiding this comment.
Import can be omitted in this implementation
There was a problem hiding this comment.
Actually, if the import is omitted, the entire try/except can bee omitted...
There was a problem hiding this comment.
Sweet! Will do.
I just put this code up quick it's by no means refined yet.
There was a problem hiding this comment.
@ksunden i'm seeing the tr.inverted() use in the except: and reading the docs for it I'm wondering do we even need our own inversion at all?
Gonna put up benchmarks against both.
|
I need to dig into the Also, there may be a faster version with It's interesting to me that this current implementation is afaict geared toward processing a tensor of 3x3 matrices, batchwise using vector opts, which we're not doing here yet, there's still this drastic improvement (at least for me)? |
| return detA | ||
|
|
||
|
|
||
| # stolen verbatim from: |
| # stolen verbatim from: | ||
| def hdinv(A): | ||
| invA = np.zeros_like(A) | ||
| detA = vdet(A) |
There was a problem hiding this comment.
can we just do a zero check here and then fail gracefully?
pyqtgraph/functions.py
Outdated
| arr = np.array([[tr.m11(), tr.m12(), tr.m13()], [tr.m21(), tr.m22(), tr.m23()], [tr.m31(), tr.m32(), tr.m33()]]) | ||
| inv = numpy.linalg.inv(arr) | ||
| inv = hdinv(arr) | ||
| # TODO: use `*np.ravel()` below instead? |
There was a problem hiding this comment.
This is faster btw.
Putting up some profile reports shortly.
|
Here's a randomly selected sample of profiling messages generated from the commit to come just after writing this. (Click to see) Log messages from a bunch of scrolling/panning interactionI find this pretty interesting because I can't see a need to compute the inverse matrix manually at all? The
EDIT 🤦 :it's all in the doc string -> floating point errors when determining if matrix is invertible, something something. Here's a possibly relevant bug tracker list. I'm suspicious this was the original actual set of issues: The latter seems to have been resolved in To me we should just drop all the |
|
Oh, and I guessed wrong, looks like |
|
OH**2, I also added singularity detection to the manual inverse calc if we decide to keep a manual version. |
|
I'm also ok to assume these CI failures are unrelated right? I see: For Linux py2.7 in the azure UI. |
| Raises an exception if tr is not invertible. | ||
|
|
||
| Note that this function is preferred over QTransform.inverted() due to |
There was a problem hiding this comment.
🤦, I guess the reason is right here lol.
Clearly need to read docs summaries better 😄
Avoid drawing a new new sticky position if the mouse hasn't moved to the next (rounded) index in terms of the scene's coordinates. This completes the "discrete-ization" of the mouse/cursor UX. Finalizing this feature helped discover and solve pyqtgraph/pyqtgraph#1418 which masssively improves interaction performance throughout the whole lib! Hide stickys on startup until cursor shows up on plot.
|
Thanks for the PR @goodboy CI is failing on all Qt4 builds; would suggest testing locally to see if you can replicate (good luck configuring Qt4 locally w/o conda). Doing a search for the issue reported in the Qt bug tracker I came up with QTBUG-52070. As the issue appears to have been fixed with newer versions of Qt, if QTransform.inverted() is faster than the numpy implementation, perhaps we should only use the numpy implementation with the when Qt4 is being utilized. So instead of try/except for numpy import error, instead do: if QT_LIB in ["pyside", "pyqt"]:
# use numpy method
else:
# use qtransform method |
@j9ac9k I peeked (very briefly) at these builds and they don't seem to be failing due to this change?
See the list I linked above (of which your issue is a member).
It's definitely faster then
Sounds good 👍 |
|
Two tests that fail: Those tests have historically been very sensitive to failure; I would be surprised if the failures are unrelated to this PR, but the way to check that would be to revert the |
|
@goodboy I think I'm about to merge that PR, so I would go ahead and rebase |
This fixes not only latency but high CPU usage when inverting a Qt transform matrix. There is a known recent issue in ``numpy`` pertaining to this: numpy/numpy#17166 (comment)
94c3153 to
f9b494d
Compare
|
@j9ac9k bumped. |
|
All in all looking much better and making me more confident that this change is nbd in terms of floating point errors. CI TLDR for y'all:
|
|
For the images used for testing, we actually keep those in a separate repo here: https://github.com/pyqtgraph/test-data As part of the test suite, we clone that repo to get the images. Frankly I hate this structure, and think the images should be part of the test data, and the test data should not be part of the I have no idea why the CI is blowing up on this suddenly though. |
In theory it could be the floating point issues? |
|
@j9ac9k so I guess just rebase again here? |
I had to do a rebase to launch the example app; ... I'm going to try and script some kind of profiling code for all the different ways we can do this. I think you doing a rebase from EDIT: scratch that, error is unrelated to this PR...I'll try and fix now-ish. EDIT2: 🤦 error was due to naming my profiling file |
|
Looked at this a bit more, and looks like https://doc.qt.io/qt-5/qtglobal.html#qFuzzyIsNull
Unfortunately the precision that method offers has been determined to be insufficient; I'm still working on some profiling code here so hopefully I'll have something soon-ish |
So if it's not the latency, then we'll need to figure out another method of measuring the resource consumption that's the concern, and see if any of the tested methods here offer sufficient improvements or not.
I'm going to be very reluctant to let users opt-in on this in a way the library supports it. The failure mode here is not going to obvious, and hits method is used in almost every part of the library. If someone wants to overwrite this, they can do so via monkey-patching the method themselves (after all, this is python, we're all adults here). import pyqtgraph as pg
def patchedInvertQTransform(transform: QTransform):
return tr.inverted()
pg.functions.invertQTransform = patchedInvertQTransformGenerally matrix inversion is pretty "expensive", and to quote my numerical methods professor in undergrad @tbewley (I'm paraphrasing a bit, it's been some time)
It may be more worthwhile to investigate even if we actually need to do these inversions, and if not, can we do other matrix operations instead.
This has been reported on a few instances in the Qt bug tracker, but from their (Qt dev's) perspective, this isn't really a bug, but working as intended; it's just that their implementation is not sufficient enough for our purposes. Here are some links to the relevant bug reports; you may get some luck chiming in on the first; the second link last update was almost 11 years ago. If you can identify a way to track the resource consumption, that would be the next step here if we're going to evaluate another method. |
The latency is still definitely a problem (one that is unacceptable for large data sets IMO) but not the original reason I noticed the issue in the first place; that was CPU overuse.
This is totally fine of course, but at some point, being someone extremely concerned with latency, I'll have overriden nearly everything in It's essentially the same discoveries as in #1478; there's a lot of If there's tradeoffs that need to be made (like fp precision in this case) for certain edge cases then why are we tuning to the pathological case?
IMO if we need to handle the special cases (high prevision views in the range I guess for me the main edge
3x3 though?
Which purposes are those exactly? |
|
@goodboy, are you using a numpy built with MKL? That could explain the multi-core usage. Try adding the following before any other imports: import mkl
mkl.set_num_threads(1) |
Don't think so? Either way probably just going to override this since I have no use cases that experience the underlying fp problem and the benefits are just too great in comparison. |
|
With only examining where this method is called briefly; I would guess try to trigger the issue with either ScatterPlotItem w/ Another potential use case is a viewbox that's way zoomed in (or out?) and querying the pixel size, it's quite possible you'll get different results there as well. In this branch, I would implement both methods, and before the function returns, compare the difference between them, given that matrix inversion of sparse matrices can be very sensitive to floating-point errors, you should expect to see some variance; question is is that difference enough to trigger different results. |
|
Why not something like the following? def invertQTransform(tr):
# try fast path first
inv, ok = tr.inverted()
if ok:
return inv
import numpy.linalg
arr = np.array([[tr.m11(), tr.m12(), tr.m13()], [tr.m21(), tr.m22(), tr.m23()], [tr.m31(), tr.m32(), tr.m33()]])
inv = numpy.linalg.inv(arr)
return QtGui.QTransform(inv[0,0], inv[0,1], inv[0,2], inv[1,0], inv[1,1], inv[1,2], inv[2,0], inv[2,1]) |
I don't think the problem is determining if it is investable and falling back to numpy if its not; but the issue is the inverse of the matrix will be off to a sufficient point that attempts at pixel perfect mappings will break. If you look at where the library uses For example, if you take numpyMatrix = np.array([[tr.m11(), tr.m12(), tr.m13()], [tr.m21(), tr.m22(), tr.m23()], [tr.m31(), tr.m32(), tr.m33()]])
itr = tr.inverted()
invertedTransformMatrix = np.array([[itr.m11(), itr.m12(), itr.m13()], [itr.m21(), itr.m22(), itr.m23()], [itr.m31(), itr.m32(), itr.m33()]])
invertedNumpyMatrix = np.linalg.inv(numpyMatrix)
assert np.testing.assert_all_close(invertedTransformMatrix, invertedNumpyMatrix) # this will likely failThis is why I would advise implementing both methods, and doing comparisons in the results of the functions that call invertQTransform within, some of those places include
there are more locations, but you should get the idea. Point being here is it must be shown that these functions would give the same results @goodboy I would suggest you modify this PR to a draft, then implement both the I recognize this is a large burden to overcome; but given this has been an issue historically, and intentional changes were made to address it, and there is nothing in any Qt changelog suggesting the behavior has changed, I have little reason to believe whatever the issue was, wouldn't re-appear if we reverted to |
What QTransform.inverted() amounts to doing is:
What numpy.linalg.inv() seems to be doing is calling lapack GESV
What you are saying is that even if the use of qFuzzyIsNull were removed, it would not be sufficient? |
My understanding was it was running If it is indeed running |
|
Certain code-paths in QTransform::inverted() apply qFuzzyIsNull() to its elements, depending on the matrix type. The generic matrix type applies qFuzzyIsNull() only to the determinant. I don't know if pyqtgraph always falls into the generic matrix case. We could avoid qFuzzyIsNull() totally perhaps by using: def invertQTransform(tr):
det = tr.determinant()
# TODO: if needed, use some other criteria to determine invertable or not
return tr.adjoint() / detThis takes twice the time of simply calling inverted() due to function call overhead. |
|
Looks like the other methods that call We can easily query the type: http://doc.qt.io/qt-5/qtransform.html#TransformationType-enum and go from there? |
|
From your transforms.txt, I counted: With your dataset, the following is slower than just unconditionally computing tr.adjoint() / tr.determinant(). def invertTransforms3():
transforms = originalTransforms.copy()
TxScale = QtGui.QTransform.TransformationType.TxScale
for tr in transforms:
if tr.type() == TxScale:
det = tr.determinant()
inv = tr.adjoint() / det
else:
inv, ok = tr.inverted()
return None |
|
We can't quite unconditionally do that division, as we would have to catch the case when the determinant is 0; ... should we then call a psedo-inverse function (they're a bit slower, but in theory they should be never called so who cares?) ...that said I can't remember ever getting a Transform is not invertable exception... |
|
Looks like I got confused myself in trying to avoid qFuzzyIsNull(). So the simplest would still be: def invertQTransform(tr):
# try fast path first
inv, ok = tr.inverted()
if ok:
return inv
import numpy.linalg
arr = np.array([[tr.m11(), tr.m12(), tr.m13()], [tr.m21(), tr.m22(), tr.m23()], [tr.m31(), tr.m32(), tr.m33()]])
inv = numpy.linalg.inv(arr)
return QtGui.QTransform(inv[0,0], inv[0,1], inv[0,2], inv[1,0], inv[1,1], inv[1,2], inv[2,0], inv[2,1])None of the ~5000 test matrices enter the slow path. |
|
Now I don't really do much C coding, but the TXScale method seems to do some checks on the m11, and m22 values: oh wait...that just checks for invertability... I would still advocate for using https://numpy.org/doc/stable/reference/generated/numpy.linalg.pinv.html That should handle the case of singlar matrices better; which is what that method would expect to get... |
|
Hmm, maybe we jumped the gun a little. To classify the kind of matrix, a call to the method type() is made. In the following, "9" is used to represent an arbitrary value. So for instance, something that is not exactly a TxScale matrix could be classified as one and thus get inverted using the TxScale inversion way. So to avoid this fuzzy matrix classification, we need to avoid calling inverted(), maybe something like the following? def invertQTransform(tr):
# try fast path first
det = tr.determinant()
bad = abs(det) <= 1e-12
if not bad:
return tr.adjoint() / det
# anything else gets passed to the old way
# TODO: use numpy.linalg.pinv
import numpy.linalg
arr = np.array([[tr.m11(), tr.m12(), tr.m13()], [tr.m21(), tr.m22(), tr.m23()], [tr.m31(), tr.m32(), tr.m33()]])
inv = numpy.linalg.inv(arr)
return QtGui.QTransform(inv[0,0], inv[0,1], inv[0,2], inv[1,0], inv[1,1], inv[1,2], inv[2,0], inv[2,1])In my opinion, not changing np.linalg.inv() to np.linalg.pinv() has the advantage that the slow path code is just a cut-and-paste of the tested original code. |
|
A synthetic case to cause QTransform to use TxScale inversion method or general inversion method depending on whether small value meets qFuzzyIsNull() criteria. import numpy as np
from PySide2 import QtGui
def nd_to_qtr(a):
return QtGui.QTransform(*a.ravel())
def qtr_to_nd(tr):
return np.array([[tr.m11(), tr.m12(), tr.m13()],
[tr.m21(), tr.m22(), tr.m23()], [tr.m31(), tr.m32(), tr.m33()]])
def test(e):
arr = np.array([[5,e,e], [e,5,0], [0,0,1]])
tr = nd_to_qtr(arr)
print(tr.type())
qt_inv = qtr_to_nd(tr.inverted()[0])
print(arr)
print(qt_inv)
print(arr @ qt_inv)
test(1e-11) # classified as TxProject
test(1e-12) # classified as TxScale |
|
Nice find on how the transforms are classified! Originally I was going to suggest trying to see how fast def invertQTransform(tr):
# try fast path first
det = tr.determinant()
bad = abs(det) <= 1e-12
if not bad:
return tr.adjoint() / det
else:
arr = np.array([[tr.m11(), tr.m12(), tr.m13()], [tr.m21(), tr.m22(), tr.m23()], [tr.m31(), tr.m32(), tr.m33()]])
det = np.linalg.det(arr)
if det != 0: # due to floating poinit math, we probably want an (abs) <= some_really_small_value here
return tr.adjoint() / det
else:
pinv = np.linalg.pinv(arr)
return QtGui.QTransform(pinv[0,0], pinv[0,1], pinv[0,2], pinv[1,0], pinv[1,1], pinv[1,2], pinv[2,0], pinv[2,1])This way we should always get an inverted transform, even when the matrix isn't invertable. |
|
In the original numpy implementation: inv[2,2] is assumed to be 1.0 and is forced to 1.0 With the changes, I got the following timings on my machine. One optimization is avoiding a general 3x3 inversion when the right-most column is exactly [0, 0, 1], which happens to be the case for all the transforms in transforms.txt. Finished numpy_orig: 406.2722 ms import collections
import fileinput
import PySide2
from PySide2.QtGui import QTransform
from pyqtgraph.debug import Profiler
import numpy as np
import scipy.linalg
# note that the original forces inv[2,2] to 1.0,
# so all our implementations need to follow this
def numpy_orig(tr):
arr = np.array([[tr.m11(), tr.m12(), tr.m13()],
[tr.m21(), tr.m22(), tr.m23()],
[tr.m31(), tr.m32(), tr.m33()]])
inv = np.linalg.inv(arr)
return QTransform(inv[0, 0], inv[0, 1], inv[0, 2],
inv[1, 0], inv[1, 1], inv[1, 2],
inv[2, 0], inv[2, 1])
def scipy_linalg(tr):
# scipy with marshalling improvements incorporated
arr = np.array([[tr.m11(), tr.m12(), tr.m13()],
[tr.m21(), tr.m22(), tr.m23()],
[tr.m31(), tr.m32(), tr.m33()]])
arr.shape = (3, 3)
inv = scipy.linalg.inv(arr)
return QTransform(*inv.ravel().tolist()[:-1])
def marshalling_overhead_orig(tr):
# measure cost of marshalling between Qt -> numpy -> Qt
inv = np.array([[tr.m11(), tr.m12(), tr.m13()],
[tr.m21(), tr.m22(), tr.m23()],
[tr.m31(), tr.m32(), tr.m33()]])
return QTransform(inv[0, 0], inv[0, 1], inv[0, 2],
inv[1, 0], inv[1, 1], inv[1, 2],
inv[2, 0], inv[2, 1])
def marshalling_overhead(tr):
# measure cost of marshalling between Qt -> numpy -> Qt
# faster to use a 1d-list than a 2d-list
arr = np.array([tr.m11(), tr.m12(), tr.m13(),
tr.m21(), tr.m22(), tr.m23(),
tr.m31(), tr.m32(), tr.m33()])
arr.shape = (3, 3)
# faster to apply a list than an ndarray
return QTransform(*arr.ravel().tolist()[:-1])
def python_general(tr):
m11, m12, m13 = tr.m11(), tr.m12(), tr.m13()
m21, m22, m23 = tr.m21(), tr.m22(), tr.m23()
m31, m32, m33 = tr.m31(), tr.m32(), tr.m33()
h11 = m22*m33 - m23*m32
h21 = m23*m31 - m21*m33
h31 = m21*m32 - m22*m31
h12 = m13*m32 - m12*m33
h22 = m11*m33 - m13*m31
h32 = m12*m31 - m11*m32
h13 = m12*m23 - m13*m22
h23 = m13*m21 - m11*m23
h33 = m11*m22 - m12*m21
# m13, m23, m33 is likely to be 0, 0, 1
det = m13*h31 + m23*h32 + m33*h33
detr = 1.0 / det # let singular matrices raise ZeroDivisionError
inv = QTransform(h11 * detr, h12 * detr, h13 * detr,
h21 * detr, h22 * detr, h23 * detr,
h31 * detr, h32 * detr)
return inv
def python_affine(tr):
m11, m12, m13 = tr.m11(), tr.m12(), tr.m13()
m21, m22, m23 = tr.m21(), tr.m22(), tr.m23()
m31, m32, m33 = tr.m31(), tr.m32(), tr.m33()
h31 = m21*m32 - m22*m31
h32 = m12*m31 - m11*m32
h33 = m11*m22 - m12*m21
if m13 == 0.0 and m23 == 0.0 and m33 == 1.0:
det = h33
detr = 1.0 / det # let singular matrices raise ZeroDivisionError
inv = QTransform(m22 * detr, -m12 * detr,
-m21 * detr, m11 * detr,
h31 * detr, h32 * detr)
return inv
else:
det = m13*h31 + m23*h32 + m33*h33
detr = 1.0 / det # let singular matrices raise ZeroDivisionError
h11 = m22*m33 - m23*m32
h21 = m23*m31 - m21*m33
# h31 = m21*m32 - m22*m31
h12 = m13*m32 - m12*m33
h22 = m11*m33 - m13*m31
# h32 = m12*m31 - m11*m32
h13 = m12*m23 - m13*m22 # zero if m13 == m23 == 0
h23 = m13*m21 - m11*m23 # zero if m13 == m23 == 0
# h33 = m11*m22 - m12*m21
inv = QTransform(h11 * detr, h12 * detr, h13 * detr,
h21 * detr, h22 * detr, h23 * detr,
h31 * detr, h32 * detr)
return inv
def qt_noclassification(tr):
det = tr.det()
detr = 1.0 / det # let singular matrices raise ZeroDivisionError
adj = tr.adjoint()
inv = adj * detr
inv = QTransform(inv.m11(), inv.m12(), inv.m13(),
inv.m21(), inv.m22(), inv.m23(),
inv.m31(), inv.m32())
return inv
def qt_noclassification_cheat(tr):
# we are cheating here because we didn't set inv[2,2] to 1.0
det = tr.det()
detr = 1.0 / det # let singular matrices raise ZeroDivisionError
adj = tr.adjoint()
return adj * detr
def check_exact_affine(transforms):
cnt = sum(tr.m13() == 0 and tr.m23() == 0 and tr.m33() == 1.0
for tr in transforms)
print('exact affine {} out of {}'.format(cnt, len(transforms)))
transforms = [eval(line.strip()) for line in fileinput.input("transforms.txt")]
transforms_copy = transforms.copy()
cnt = collections.Counter(tr.type() for tr in transforms_copy)
print(cnt)
check_exact_affine(transforms)
profiler = Profiler(disabled=False, delayed=False)
[marshalling_overhead_orig(tr) for tr in transforms]
profiler("Finished marshalling_overhead_orig")
[marshalling_overhead(tr) for tr in transforms]
profiler("Finished marshalling_overhead")
results = []
inv = [numpy_orig(tr) for tr in transforms]
results.append(inv)
profiler("Finished numpy_orig")
inv = [scipy_linalg(tr) for tr in transforms]
results.append(inv)
profiler("Finished scipy_linalg")
inv = [python_general(tr) for tr in transforms]
results.append(inv)
profiler("Finished python_general")
inv = [python_affine(tr) for tr in transforms]
results.append(inv)
profiler("Finished python_affine")
inv = [qt_noclassification(tr) for tr in transforms]
results.append(inv)
profiler("Finished qt_noclassification")
inv = inverted = [qt_noclassification_cheat(tr) for tr in transforms]
results.append(inv)
profiler("Finished qt_noclassification_cheat")
inv = [tr.inverted()[0] for tr in transforms]
results.append(inv)
profiler("Finished Qt inverted()")
for inverted in results:
check_exact_affine(inverted) |
I'm fairly sure this is a byproduct of the matrix math involved with the Givens rotation matrix
It's been a while since I studied this linear algebra, and while we could depend on this for for 2D; we absolutely cannot depend on this in the case of 3D operations. That said, I'm not sure how these transforms are created in 3D space; my argument is strictly of an academic nature (I'm not even sure that this is method is called in 3D operations or not). One thing that never dawned on me is to ignore numpy altogether; I keep forgetting how fast the python math module can be for singular values. Nice thought. I'm of the opinion that the implementation should be like |
The python_affine() implementation is in fact fully general for 3x3 matrices. It just avoids the redundant calculations when the right-most column is tested to be exactly [0, 0, 1]. QtGui.QTransform is purely for 2D transforms (using a 3x3 double precision matrix) [https://doc.qt.io/qt-5/qtransform.html] Qt3DCore::QTransform is for 3D transforms (using a 4x4 single precision matrix) [https://doc.qt.io/qt-5/qt3dcore-qtransform.html]
So something like this? It's slightly faster now by doing an in-place multiplication. def pinv_fallback(tr):
arr = np.array([tr.m11(), tr.m12(), tr.m13(),
tr.m21(), tr.m22(), tr.m23(),
tr.m31(), tr.m32(), tr.m33()])
arr.shape = (3, 3)
pinv = np.linalg.pinv(arr)
return QtGui.QTransform(*pinv.ravel().tolist())
def qt_noclassification_cheat(tr):
# we are cheating here because we didn't set inv[2,2] to 1.0
try:
det = tr.det()
detr = 1.0 / det # let singular matrices raise ZeroDivisionError
inv = tr.adjoint()
inv *= detr
return inv
except ZeroDivisionError:
return pinv_fallback(tr) |
|
Ahh makes sense 3d transformation matrix would be a 4x4.
Yeah, something like that; why are you defining the shape tho? |
Based on benchmarking marshalling_overhead(), that's the faster way of converting from Qt to ndarray. Not that it matters much here with pinv() dominating. |
This fixes not only latency but high CPU usage when inverting a Qt transform matrix.
There is a known recent issue in
numpypertaining to this:numpy/numpy/issues/17166
I've ripped off the "hard coded" solution from that issue verbatim and using it has reduced both latency and CPU usage in pretty much all mouse related interaction with
ViewBox.I'm happy to write up profiling and test code for this in case there are any doubts.
There are even faster solutions inside the referenced issue that might be worth exploring as well; I know we have to keep py2 in mind but it might be worth some flag logic in this case since
invertQTransform()seems to be used throughoutViewBoxandGraphicsItem. This would explain why I'm seeing improvements outside my original use case (callingGraphicsObject.mapFromView()on label coordinates fed from the mouse).I also would like to benchmark using
*np.ravel()to pass the matrix results toQtGui.QTransform()since I've done that with good results inQPicture()drawings and it should in theory be faster then the existing multiple element access.Update
Commit that changed away from using the internal transform is a41d330
Relevant Qt bug reports are: