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.

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

Application bug fixes and performance enhancements #39

Merged

Conversation

scottwittenburg
Copy link
Collaborator

@scottwittenburg scottwittenburg commented Dec 7, 2020

This PR fixes several issues with the application:

  • Use itk.js WorkerPool to speed up loading
  • Fix issues encountered when interrupting currently loading experiment
  • Support export of original CSV format
  • Work around issue using older python 3.5 until we can upgrade python in the docker image stack
  • Fix load state indicators:
    • Previously scans were marked as loaded or not loaded using filled vs. empty disc next to each scan. The computation for this was done efficiently, but was ultimately incorrect as the scan was marked as loaded as soon as a worker had picked up the last image to load. In cases where the only image in a scan is really large, the scan could be marked as loaded before it was actually ready.
    • This change removes the filled circles indicating individual scans are loaded or not in favor of a single icon located next to the experiment name.

Closes #18. Closes #42.

@dzenanz
Copy link
Member

dzenanz commented Dec 7, 2020

This dragged in all the commits from #35, because it has not been merged yet.

@scottwittenburg scottwittenburg changed the title Perf itkjs workerpool WIP: Perf itkjs workerpool Dec 7, 2020
@scottwittenburg
Copy link
Collaborator Author

Yes, thanks @dzenanz, if this becomes good enough to consider truly adding, I'll either tack it on to the end of that PR, or else rebase this on master (after that's merged).

@dzenanz
Copy link
Member

dzenanz commented Dec 7, 2020

Time to rebase :D

@scottwittenburg scottwittenburg marked this pull request as draft January 6, 2021 21:21
@scottwittenburg scottwittenburg changed the title WIP: Perf itkjs workerpool Perf itkjs workerpool Jan 6, 2021
@scottwittenburg
Copy link
Collaborator Author

This will need the cancel api addition to the WorkerPool in this PR to itk.js when it is merged and released.

Previously scans were marked as loaded or not loaded using filled
vs. empty disc next to each scan.  The computation for this was done
efficiently, but was ultimately incorrect as the scan was marked as
loaded as soon as a worker had picked up the last image to load.  In
cases where the only image in a scan is really large, the scan could
be marked as loaded before it was actually ready.

This change removes the filled circles indicating individual scans
are loaded or not in favor of a single icon located next to the
experiment name.
@scottwittenburg scottwittenburg marked this pull request as ready for review February 24, 2021 18:34
@scottwittenburg scottwittenburg changed the title Perf itkjs workerpool Application bug fixes and performance enhancements Feb 24, 2021
v =>
v.endsWith('.json') ||
v.endsWith('.csv') ||
'Needs to be a json file'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought I had replaced this with "Needs to be a json or csv file", but I must have missed it. I'll fix it before we merge.

Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Code looks good after a short look.

@aashish24
Copy link
Contributor

aashish24 commented Feb 25, 2021

itk-js + miqa = big smile :)

* Omit the "reviewer" text field
* Enable the "Bad" button as soon as a character is entered in the note
* Move the percent-loaded indicator in between double chevrons
* Overwrite any existing notes/ratings upon importing a CSV
Copy link

@jimklo jimklo left a comment

Choose a reason for hiding this comment

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

See comments.

@@ -9,7 +9,7 @@ RUN apt-get update && apt-get install -qy \
libsasl2-dev && \
apt-get clean && rm -rf /var/lib/apt/lists/*

RUN wget https://bootstrap.pypa.io/get-pip.py && python3 get-pip.py
RUN wget https://bootstrap.pypa.io/3.5/get-pip.py && python3 get-pip.py
Copy link

Choose a reason for hiding this comment

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

This URL causes an error on build.

Should be https://bootstrap.pypa.io/pip/3.5/get-pip.py instead.

@scottwittenburg
Copy link
Collaborator Author

@jeffbaumes This is the PR I was just mentioning

Only reviewer and above (reviewer, manager, admin) shouild be able to
see the widgets that allow editing, like the rating buttons and note
edit field.  Also, make the 'Bad' button visible to anyone who can see
the rest of the edit widgets.
Every time we run "docker-compose up", the provisioning script clobbers
some of the email server settings which were previously configured correctly.
This change puts those values into the .env.template file, so the user can
set them in their .env file, and then provisions with those values instead,
so once the correct values are set, they can be preserved across restarts.
@jeffbaumes jeffbaumes added this to In Progress in Sprint Board (4 week) Mar 26, 2021
@scottwittenburg scottwittenburg merged commit cb126be into OpenImaging:master Apr 5, 2021
Sprint Board (4 week) automation moved this from In Progress to Done Apr 5, 2021
@scottwittenburg scottwittenburg deleted the perf-itkjs-workerpool branch April 5, 2021 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
5 participants