-
Notifications
You must be signed in to change notification settings - Fork 154
BCI Application - moving exponential average and alternate visualization #1380
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
base: main
Are you sure you want to change the base?
Changes from all commits
fcc2cfa
89540b3
c5d03a9
ab0c1be
eaa7c33
be3aa7b
9642449
bf68818
a462d0e
46036f6
f8e8261
7d81690
cbe5351
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -24,9 +24,16 @@ ARG DEBIAN_FRONTEND=noninteractive | |||||||
| # Install curl for downloading data files | ||||||||
| RUN apt-get update && apt-get install -y curl && rm -rf /var/lib/apt/lists/* | ||||||||
|
|
||||||||
| # Install python3-tk (needed for matplotlib GUI support) | ||||||||
| ARG WITH_MATPLOTLIB=no | ||||||||
| RUN if [ "$WITH_MATPLOTLIB" = "yes" ]; then apt-get update && apt-get install -y python3-tk && rm -rf /var/lib/apt/lists/*; fi | ||||||||
|
|
||||||||
| ENV HOLOSCAN_INPUT_PATH=/workspace/holohub/data/bci_visualization | ||||||||
|
|
||||||||
| # Install Python dependencies | ||||||||
| COPY applications/bci_visualization/requirements.txt /tmp/requirements.txt | ||||||||
| RUN pip install -r /tmp/requirements.txt --no-cache-dir | ||||||||
|
|
||||||||
|
|
||||||||
| # Install optional matplotlib dependencies | ||||||||
| COPY applications/bci_visualization/requirements-matplotlib.txt /tmp/requirements-matplotlib.txt | ||||||||
| RUN if [ "$WITH_MATPLOTLIB" = "yes" ]; then pip install -r /tmp/requirements-matplotlib.txt --no-cache-dir; fi | ||||||||
|
bhashemian marked this conversation as resolved.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -69,9 +69,26 @@ | |||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| "run": { | ||||||||||||||||||||||||||||||||||||||||||
| "command": "python3 bci_visualization.py --renderer_config <holohub_app_source>/config.json --mask_path <holohub_data_dir>/bci_visualization/anatomy_labels_high_res.nii.gz", | ||||||||||||||||||||||||||||||||||||||||||
| "workdir": "holohub_app_source" | ||||||||||||||||||||||||||||||||||||||||||
| "default_mode": "clara_viz", | ||||||||||||||||||||||||||||||||||||||||||
| "modes": { | ||||||||||||||||||||||||||||||||||||||||||
| "clara_viz": { | ||||||||||||||||||||||||||||||||||||||||||
| "description": "Run the application with ClaraViz for volume rendering.", | ||||||||||||||||||||||||||||||||||||||||||
| "run": { | ||||||||||||||||||||||||||||||||||||||||||
| "command": "python3 bci_visualization.py --renderer_config <holohub_app_source>/config.json --mask_path <holohub_data_dir>/bci_visualization/anatomy_labels_high_res.nii.gz", | ||||||||||||||||||||||||||||||||||||||||||
| "workdir": "holohub_app_source" | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| "matplotlib": { | ||||||||||||||||||||||||||||||||||||||||||
| "description": "Run the application with matplotlib for volume rendering.", | ||||||||||||||||||||||||||||||||||||||||||
| "build": { | ||||||||||||||||||||||||||||||||||||||||||
| "docker_build_args": ["--build-arg", "WITH_MATPLOTLIB=yes"] | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| "run": { | ||||||||||||||||||||||||||||||||||||||||||
| "command": "python3 bci_visualization.py --renderer_config <holohub_app_source>/config.json --mask_path <holohub_data_dir>/bci_visualization/anatomy_labels_high_res.nii.gz --viz matplotlib", | ||||||||||||||||||||||||||||||||||||||||||
| "workdir": "holohub_app_source" | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+81
to
+90
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Description is inaccurate - matplotlib doesn't perform volume rendering. The description states "matplotlib for volume rendering" but matplotlib is a 2D plotting library. Based on the PR summary mentioning "surface mesh rendering", consider updating the description to accurately reflect the visualization approach. 📝 Suggested fix "matplotlib": {
- "description": "Run the application with matplotlib for volume rendering.",
+ "description": "Run the application with matplotlib for surface mesh visualization.",📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trailing whitespace before closing brace Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
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 would be great to move this to the end to have all matplotlib related dependencies in one place. In this way, both images, with and without matplotlib, can share the initial cached docker layers. Thanks