Skip to content

add fastpath for float images with nans#2970

Merged
j9ac9k merged 3 commits intopyqtgraph:masterfrom
pijyoi:imgitem-float-nan
Apr 17, 2024
Merged

add fastpath for float images with nans#2970
j9ac9k merged 3 commits intopyqtgraph:masterfrom
pijyoi:imgitem-float-nan

Conversation

@pijyoi
Copy link
Copy Markdown
Contributor

@pijyoi pijyoi commented Mar 23, 2024

This PR augments #1693 by adding support for float images containing nans.

To test it out.

set PYQTGRAPHPROFILE="ImageItem.paint"

Then run the following script and change the levels using the HistogramLUTItem. The GUI should feel more responsive with this PR versus master.

import pyqtgraph as pg
import numpy as np
pg.setConfigOptions(imageAxisOrder='row-major')
# pg.setConfigOptions(useNumba=True)

Y = np.sinc(np.linspace(-6, 10, 4000))[:, np.newaxis]
X = np.sinc(np.linspace(-15, 9, 6000))
data = np.abs(Y * X)
data[data > 0.95] = -1
data = 10*np.log10(data)
data = data.astype(np.float32)

pg.mkQApp()
imv = pg.ImageView()
imv.show()
imv.setImage(data, levels=(-50, 0))
hlut = imv.getHistogramWidget()
hlut.gradient.setColorMap(pg.colormap.get("CET-C1"))
hlut.gradient.showTicks(False)
pg.exec()

timings from PYQTGRAPHPROFILE

row-major master row-major this PR col-major master col-major this PR
numba 224 ms 35 ms 558 ms 165 ms
numpy 258 ms 128 ms 303 ms 252 ms

numba codepath with col-major is no longer worse than numpy. If you were interested in performance, you wouldn't be using col-major anyway...

@pijyoi pijyoi force-pushed the imgitem-float-nan branch 5 times, most recently from 48dd6aa to 5ed6c93 Compare March 24, 2024 03:45
@pijyoi pijyoi marked this pull request as ready for review March 24, 2024 09:38
Copy link
Copy Markdown
Member

@j9ac9k j9ac9k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @pijyoi

As always, really appreciate the comments, it helps a ton as I read through this.

My only comment is regarding the assert statements, which can easily be bypassed if running Python with the -O argument. Outside of test code we should probably just have an if check and raise an exception of some kind (I don't have strong opinions on what exceptions to raise but maybe one of the numpy ones?)

if lut.ndim == 1:
lut = lut[:, xp.newaxis]

assert lut.ndim == 2 and lut.shape[1] in (1, 3, 4)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is an assert statement what we actually want here? These are bypassed when Python is run with the -O argument.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not so sure that this makes more sense as an exception. Sometimes I use assertions as comments to document the code flow.

I converted it to an exception but I moved it to the top of the function, otherwise it doesn't make sense to raise a complaint that the lut was not 2d.

# instead use it as an Indexed8 ColorTable. This is only
# applicable if the lut has <= 256 entries.

assert (not forceApplyLut) or (lut is not None)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment about assert as earlier.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one makes sense to be an exception.

@pijyoi pijyoi marked this pull request as draft March 28, 2024 11:35
@pijyoi pijyoi force-pushed the imgitem-float-nan branch 2 times, most recently from f6985de to 9196ac5 Compare March 28, 2024 17:41
@pijyoi pijyoi marked this pull request as ready for review March 29, 2024 14:47
@pijyoi pijyoi force-pushed the imgitem-float-nan branch from 058310a to 157f89a Compare March 30, 2024 05:06
@pijyoi
Copy link
Copy Markdown
Contributor Author

pijyoi commented Mar 31, 2024

While improving the numba performance for the float with nans case, we also found out how to improve the numba performance for the float with no nans case. (24ms --> 12ms)

The gap between numpy vs numba has widened; and the gap between numba and cupy has narrowed.

testable with:

VideoSpeedTest.py --dtype float --size 6000x4000 --levels 0,2

On a particular machine, improvement was from 38 fps to 57 fps. (50% improvement)

numpy numba cupy
21 fps 57 fps 53 fps

@pijyoi
Copy link
Copy Markdown
Contributor Author

pijyoi commented Apr 2, 2024

Revisiting the "optimized" rescaleData for uint16, it has now turned into a pessimization.
Remove the code as it is difficult to maintain for no benefit.

Surprisingly, for an uint16 image the same size as the float32 image above, the float32 image is running (slightly) faster.

import pyqtgraph as pg
import numpy as np

pg.setConfigOptions(imageAxisOrder='row-major')
# pg.setConfigOptions(useNumba=True)

Y = np.sinc(np.linspace(-6, 10, 4000))[:, np.newaxis]
X = np.sinc(np.linspace(-15, 9, 6000))
data = np.abs(Y * X)

data *= 2000
data = data.astype(np.uint16)

pg.mkQApp()
imv = pg.ImageView()
imv.show()
imv.setImage(data, levels=(0.0, 1000.0))
hlut = imv.getHistogramWidget()
hlut.gradient.setColorMap(pg.colormap.get("CET-C1"))
hlut.gradient.showTicks(False)
pg.exec()
print(imv.imageItem.qimage.format())

@pijyoi pijyoi force-pushed the imgitem-float-nan branch from d92fa55 to d9e0cd6 Compare April 2, 2024 14:50
@pijyoi
Copy link
Copy Markdown
Contributor Author

pijyoi commented Apr 2, 2024

Change uint16 mono images to use functions_numba.rescale_only instead of functions_numba.rescaleData.

VideoSpeedTest.py --dtype uint16 --size 6000x4000 --levels 0,10000

On a particular machine, improvement was from 46 fps to 83 fps. (80% improvement)

numpy numba cupy
22 fps 83 fps 55 fps

@pijyoi pijyoi force-pushed the imgitem-float-nan branch from d9e0cd6 to 63f4f27 Compare April 3, 2024 13:55
@pijyoi
Copy link
Copy Markdown
Contributor Author

pijyoi commented Apr 6, 2024

Added support for float RGB images with nans.
The supported data types can thus be summarized as (uint8, uint16, float32, float64) x (1 chan, 3 chans) provided no multi-channel levels are used.

import pyqtgraph as pg
import numpy as np
pg.setConfigOptions(imageAxisOrder='row-major')
# pg.setConfigOptions(useNumba=True)

bias = 50

Y = np.sinc(np.linspace(-6, 10, 4000))[:, np.newaxis]
X = np.sinc(np.linspace(-15, 9, 6000))
data = np.abs(Y * X)
mask1 = data > 0.95
mask2 = (0.5 < data) & (data < 0.6)
data = 10*np.log10(data)
data += bias
data = np.dstack((data, data * 0.5, data * 0.25))
data[..., 1][mask1] = np.nan
data[..., 2][mask2] = np.nan
data = data.astype(np.float32)

pg.mkQApp()
imv = pg.ImageView(levelMode='mono')
imv.show()
imv.setImage(data, levels=(-50 + bias, 0 + bias))
pg.exec()
print(imv.imageItem.qimage.format())

@pijyoi pijyoi force-pushed the imgitem-float-nan branch from a34d755 to d5e2f6b Compare April 13, 2024 09:14
@pijyoi pijyoi changed the title add fastpath for float 2d images with nans add fastpath for float images with nans Apr 13, 2024
pijyoi added 3 commits April 13, 2024 21:30
- don't restrict fastpath to c_contiguous
- don't handle scaling of rgba images
- update RawImageWidget to handle float images with nans
for some reason, rewriting the function using np.nditer runs 2x as
fast as the guvectorize version.

uint16 mono image: make use of the improved numba rescale_and_clip
with improvements in numpy, this code has become a pessimization
instead.

furthermore, the code is difficult to maintain.
@pijyoi pijyoi force-pushed the imgitem-float-nan branch from d5e2f6b to 9ee68b5 Compare April 13, 2024 13:31
@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Apr 17, 2024

that numba rescale_and_clip function seems to be exactly what's needed haha, wow, that's really fast.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Apr 17, 2024

Every time I think you've gotten as much out of ImageItem performance as we can get, you prove me wrong 😆 Thanks for the PR @pijyoi Merging.

@j9ac9k j9ac9k merged commit 3d15f22 into pyqtgraph:master Apr 17, 2024
@pijyoi pijyoi deleted the imgitem-float-nan branch April 17, 2024 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants