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

Make PlotPeaks robust enough that we don't need --writable-tempfs in singularity/apptainer #174

Merged
merged 4 commits into from
Nov 20, 2024

Conversation

mattcieslak
Copy link
Contributor

@mattcieslak mattcieslak commented Nov 19, 2024

Attempt to manage xvfb outside of nipype so we can use a writable temp dir. We've had so much trouble with FURY plotting that we added the --skip-odf-reports to bypass them. But they are pretty and useful, so it would be great to get them working robustly.

There is an error that isn't captured in the logs/stdout when trying to run qsirecon with --containall in singularity/apptainer. All we typically see is that node plot_peaks failed to run and that its output was not found.

Some things we know:

  • FURY Plotting always works in Docker
  • If we remove _redirect_x variable from the interface, FURY plotting doesn't work. even in a container (eg c08a3f7)
  • FURY plotting sometimes doesn't work in singluarity/apptainer. This is probably due to different system defaults on different clusters
  • Plotting reliably works on singularity/apptainer with --writable-tempfs
  • We think maybe the issue has to do with the temp directory used by xvfb. I tried changing this in [FIX] set TMPDIR before running plot_peaks #163, but @araikes tested and it causes a new error

@mattcieslak mattcieslak added the bug Something isn't working label Nov 19, 2024
@codecov-commenter
Copy link

codecov-commenter commented Nov 20, 2024

Codecov Report

Attention: Patch coverage is 0% with 25 lines in your changes missing coverage. Please review.

Project coverage is 47.73%. Comparing base (01e31d6) to head (c6e8eac).

Files with missing lines Patch % Lines
qsirecon/cli/recon_plot.py 0.00% 25 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #174      +/-   ##
==========================================
- Coverage   47.86%   47.73%   -0.14%     
==========================================
  Files          57       57              
  Lines        7195     7213      +18     
  Branches      983      984       +1     
==========================================
- Hits         3444     3443       -1     
- Misses       3546     3565      +19     
  Partials      205      205              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@mattcieslak mattcieslak requested a review from tsalo November 20, 2024 00:02
@mattcieslak
Copy link
Contributor Author

@araikes I wasn't able to request you as a reviewer for some reason, but could you take a look too?

The plan here is to at the bare minimum give users a more clear error message about why PlotPeaks is failing. I'm not sure we're going to be able to get around the Xvfb issues

@tsalo tsalo merged commit 3d14e53 into main Nov 20, 2024
24 checks passed
@tsalo tsalo deleted the fix/xvbf-outside-nipype branch November 20, 2024 16:08
@tsalo tsalo mentioned this pull request Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants