-
Notifications
You must be signed in to change notification settings - Fork 38
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
Simplifying to a single file, perf improvements, better user experience #7
base: main
Are you sure you want to change the base?
Conversation
- allocating arrays ahead of time to save mem/improve speed - prebuilding graph outside of main loop - prebuilding rgb associated colors for calibrated data
|
||
cv2.imshow(title2,waterfall_vertical) | ||
|
||
while 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.
Put this in a loop so that if I hold down t
or g
to adjust the gain, it responds smoothly.
# from skewing measurements below | ||
cap.isOpened() | ||
|
||
def runall(): |
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 turned this into a function so that I could more easily profile the logic.
src/PySpectrometer2.py
Outdated
result.append(rgb) | ||
return result | ||
|
||
wavelength_data_rgbs = compute_wavelength_rgbs(wavelengthData) |
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.
Precomputing rgb values outside the main while loop for speed/memory.
@@ -188,18 +193,57 @@ def handle_mouse(event,x,y,flags,param): | |||
tens = (graticuleData[0]) | |||
fifties = (graticuleData[1]) | |||
|
|||
# load the banner image once |
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.
Preload the banner once outside the loop.
|
||
return result | ||
|
||
graph_base = build_graph_base() |
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.
Build the background of the display once, this way we can use np.copy
to just overwrite the display at the start of each loop which is much faster and uses less memory.
# reset the graph to the base | ||
np.copyto(graph, graph_base) | ||
|
||
num_mean = 3 # average the data from this many rows of pixels |
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.
This means you could possibly make the number of rows that are averaged a passable argument.
mouseX = x | ||
mouseY = y-mouseYOffset | ||
clickArray.append([mouseX,mouseY]) | ||
if len(peak_intensities) > 0: |
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'm not sure if selecting a peak within +/- 5 pixels is better, or if choosing the closest label is. I've left +/-5 on, but maybe this could also be an option enabled by passable arguments?
|
||
return peaks | ||
|
||
def compute_wavelengths(width, pixels, wavelengths, errors): |
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.
Splitting readcal
in two. This way we can call the function separately elsewhere.
I use this when calibrating a set of points so that I can output the error of the current selection. That way I can choose to update the caldata file or undo my changes if I've managed to do worse than my previous calibration.
pxdata = ','.join(map(str, pxdata)) #convert array to string | ||
wldata = ','.join(map(str, wldata)) #convert array to string | ||
with open('caldata.txt','w') as f: | ||
f.write(str(frameWidth)+'\r\n') |
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.
Include the current frameWidth with the data. This way if the frame size is later changed, the calibration data still applies.
#find peaks and label them | ||
textoffset = 12 | ||
|
||
np.clip(intensity, 0, 255, out=intensity) |
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 numpy to draw the intensity data using vector ops over python loops.
dy[plateau[plateau < median]] = dy[plateau[0] - 1] | ||
# set rightmost and middle values to rightmost non zero values | ||
dy[plateau[plateau >= median]] = dy[plateau[-1] + 1] | ||
lplat = plateau.shape[0] |
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 noticed that the data in plateau
is always in sorted order, so we can skip finding medians and know that the middle element is the median.
right = dy[plateau[lplat-1] + 1] | ||
middle = lplat // 2 | ||
# always in sorted order | ||
median = plateau[middle] |
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.
It's actually faster to just loop over the plateau
elements once and do something with each element than to do the vectorized ops they were performing before. The changes in this function result in a roughly 4+x speedup which is awesome because the peak finding logic is a big cause of slowness when running the loop.
I've broken up my changes into several commits to make it easier to understand what's going on.
I've combined the picam and usb independent files into a single file, that way any code changes done won't have to be replicated over multiple files. README updated to reflect the change. Basically pass
--picam
,--webcam
, or--device X
to choose your target.Next up I made it possible to change the
frameWidth
andframeHeight
without breaking the code. That way i can make the frame bigger so that selecting a specific pixel becomes easier.After that we have several performance improvements. I basically profiled for both speed and memory, and made changes to address both. Nothing too crazy, just moving some logic out of the main loop that can be computed once and reused from within the loop. Also trading for loops for builtin numpy functions in spots, and finally reusing arrays instead of allocating new ones. Note: to simplify the review of a particular change here, I'd add
?w=1
to the end of the url above. That ignores whitespace changes, and there are a bunch of lines that were indented which will make it harder to review (IMO).Finally I added logic when selecting a point to do calibration that looks for the nearest peak (within 5 places at the moment). That way if your abilities with a mouse are rubbish, we'll help you select the peak you were aiming for. Right-clicking undoes the last selection in case you make a mistake.
Let me know if you have any questions. Perf boosts result in a roughly 1.8x FPS increase on my Pi4, which makes the experience a bit smoother. I mainly started doing the perf changes because I increased the
frameWidth
andframeHeight
which slows down cv2s drawing speed, making the experience less pleasant.