Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Live Viewer calculates intensity of live images via QThreads #2457

Merged
merged 26 commits into from
Jan 23, 2025
Merged

Live Viewer calculates intensity of live images via QThreads #2457

merged 26 commits into from
Jan 23, 2025

Conversation

MikeSullivan7
Copy link
Collaborator

@MikeSullivan7 MikeSullivan7 commented Jan 14, 2025 •
edited by samtygier-stfc
Loading

Issue

Closes #2448

Description

The calculation of the Intensity Profile in the Live Viewer is now done asynchronously via a QThread and the GUI is updated via the main thread upon the QThread termination. The intensity is calculated in chunks of 100 points, each done via a different thread. It is done in this way so that the GUI will remain responsive and smooth while the ROI is moved on the image, all while the intensity updates to the new position. The calculation speed is also increased by using the ImageCache class.
Unit and System tests have been added for the Live Viewer.

This PR will require PR #2425 after which this PR will need to be rebased.

Testing

make check
make test-system

Acceptance Criteria

Check that all tests pass.

  1. Open Live Viewer
  2. Right click -> Operations -> Calculate Intensity
  3. Ensure that the Intensity plot opens, the ROI is visible, and that the intensity is plotted correctly (check against known data)
  4. Move and resize the ROI and check that the Intensity plot updates smoothly with no GUI interruption.

Documentation

Release Note

@MikeSullivan7 MikeSullivan7 force-pushed the 2448_live_viewer_spectrum_async branch from cc43f76 to c53660e Compare January 14, 2025 15:49
@coveralls
Copy link

coveralls commented Jan 14, 2025 •
edited
Loading

Coverage Status

coverage: 73.114% (-0.5%) from 73.612%
when pulling 660ec9a on 2448_live_viewer_spectrum_async
into dd5949f on main.

@MikeSullivan7 MikeSullivan7 marked this pull request as ready for review January 15, 2025 13:29
@MikeSullivan7 MikeSullivan7 changed the title 2448 live viewer spectrum async Live Viewer calculates intensity of live images via QThreads Jan 15, 2025
samtygier-stfc
samtygier-stfc requested changes Jan 20, 2025
View reviewed changes
Copy link
Collaborator

@samtygier-stfc samtygier-stfc left a comment

Choose a reason for hiding this comment

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

Looking good from a first pass, here are some requests for the code.

mantidimaging/gui/test/gui_system_base.py Outdated Show resolved Hide resolved
mantidimaging/gui/windows/live_viewer/live_view_widget.py Outdated Show resolved Hide resolved
mantidimaging/gui/windows/live_viewer/live_view_widget.py Outdated Show resolved Hide resolved
mantidimaging/gui/windows/live_viewer/model.py Outdated Show resolved Hide resolved
mantidimaging/gui/windows/live_viewer/model.py Outdated Show resolved Hide resolved
mantidimaging/gui/windows/live_viewer/model.py Outdated Show resolved Hide resolved
mantidimaging/gui/windows/live_viewer/model.py Outdated Show resolved Hide resolved
mantidimaging/gui/windows/live_viewer/model.py Outdated Show resolved Hide resolved
mantidimaging/gui/windows/live_viewer/presenter.py Outdated Show resolved Hide resolved
mantidimaging/gui/windows/live_viewer/test/model_test.py Outdated Show resolved Hide resolved
@MikeSullivan7 MikeSullivan7 force-pushed the 2448_live_viewer_spectrum_async branch from 15d707b to 9aca1b6 Compare January 21, 2025 11:31
@MikeSullivan7 MikeSullivan7 self-assigned this Jan 21, 2025
@MikeSullivan7 MikeSullivan7 mentioned this pull request Jan 21, 2025
Open
samtygier-stfc
samtygier-stfc requested changes Jan 21, 2025
View reviewed changes
Copy link
Collaborator

@samtygier-stfc samtygier-stfc left a comment

Choose a reason for hiding this comment

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

Looking good. I couple of code nitpicks.

I hit a couple of issues when running the simulate script in slow mode --mode slow, but I'd be happy to merge this and fix those after.

mantidimaging/eyes_tests/live_viewer_window_test.py Outdated Show resolved Hide resolved
mantidimaging/gui/windows/live_viewer/model.py Outdated Show resolved Hide resolved
mantidimaging/gui/windows/live_viewer/live_view_widget.py Outdated Show resolved Hide resolved
@MikeSullivan7 MikeSullivan7 force-pushed the 2448_live_viewer_spectrum_async branch 8 times, most recently from ebfbdb0 to 25001e0 Compare January 22, 2025 17:06
@MikeSullivan7
Live Viewer Spectrum added
dac9a88
@MikeSullivan7
Mean buffer loading and prints removed
bbcc2f1
@MikeSullivan7
mean calculated with incoming data and displayed, base ImageCache cla…
85e6991 
…ss created
@MikeSullivan7
LV Model add and stores mean in dict, replots when ROI is moved & tes…
b7610db 
…t fixes
@MikeSullivan7
New mean array plotted when ROI is starting to move
d4fda4d
@MikeSullivan7
check if mean for given image path has been added to prevent duplication
69ff45b
@MikeSullivan7
mean of cached imaged calculated with ROI is moved around
f9542a3
@MikeSullivan7
When ROI is moved, mean of cache is shown immediately, then images ar…
fd7f24b 
…e buffered into the spectrum
@MikeSullivan7
Store Image_Data obj in cache and added TODOs
ece71a2
@MikeSullivan7
Model no longer directly accesses image_cache, mean added in presente…
af8b5bd 
…r.update_image_list
@MikeSullivan7
Added Unit Tests for Live Viewer
c4c0706
@MikeSullivan7
removed unneeded get methods in ImageCache
c6c01a2
@MikeSullivan7
spectrum is updated asyncronously via a Thread (with warnings)
9b2b45e
@MikeSullivan7
Mean is calculated in separate thread, spectrum updated in Main thread
6d16945
@MikeSullivan7
cleanup and suggested changes
5d54bbf
@MikeSullivan7
roi is disabled while spectrum is calculating to prevent Qthread bein…
be92475 
…g deleted prematurely
@MikeSullivan7
using QTimer to notify LV to recalcualte and plot spectrum asynchrono…
2fbda32 
…usly
@MikeSullivan7
set_roi_enabled refactor
55c9456
@MikeSullivan7
mypy type annotations
812b52e
@MikeSullivan7
unit tests added: test_WHEN_handle_notify_roi_moved_THEN_timer_starte…
cccd26c 
…d, test_WHEN_roi_set_enabled_THEN_roi_enabled, test_WHEN_nans_in_mean_THEN_handle_roi_change_timer_start, test_WHEN_no_nans_in_mean_THEN_handle_roi_change_timer_not_started
@MikeSullivan7
renaming spectrum to intensity profile
1e8e7b6
@MikeSullivan7
Added Live Viewer System Tests
ad43400
@MikeSullivan7
release note
6a69db6
@MikeSullivan7
pass loading function from presenter to ImageCache
13f7600
@MikeSullivan7
removed unused methods in the Model
0fbf5eb
@MikeSullivan7
Changes requested Part 1
660ec9a
@MikeSullivan7 MikeSullivan7 force-pushed the 2448_live_viewer_spectrum_async branch from 25001e0 to 660ec9a Compare January 23, 2025 15:26
samtygier-stfc
samtygier-stfc approved these changes Jan 23, 2025
View reviewed changes
Copy link
Collaborator

@samtygier-stfc samtygier-stfc left a comment

Choose a reason for hiding this comment

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

Looks good. I have tested locally.

There are a few issues to be done as separate PRs, #2469 #2474

@samtygier-stfc samtygier-stfc added this pull request to the merge queue Jan 23, 2025
Merged via the queue into main with commit e6f10f6 Jan 23, 2025
8 checks passed
@samtygier-stfc samtygier-stfc deleted the 2448_live_viewer_spectrum_async branch January 23, 2025 16:21
@JackEAllen JackEAllen mentioned this pull request Jan 24, 2025
Closed
@MikeSullivan7 MikeSullivan7 mentioned this pull request Feb 12, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samtygier-stfc samtygier-stfc samtygier-stfc approved these changes

Assignees

@MikeSullivan7 MikeSullivan7

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Calculate Live Viewer Spectrum asynchronously
3 participants
@MikeSullivan7 @coveralls @samtygier-stfc