Move some imports to make GPU optional#346
Conversation
|
Hey @MoritzWM, Thanks for the PR! I understand your use case, did you test that this is all it takes for your jobs to run? Could you tell me:
First option is so I can think about what tests to add to make sure this doesn't break in the future |
|
seems like our pre-commit checks are failing, do you mind:
|
|
Hello @sroet, sorry forgot to do the most basic things. Fixed the formatting. Yes it seems that this is all that is required. I didn't test all options though, only a basic set. I will test a little more tomorrow to really make sure.
Only pytom_extract_candidates.py and pytom_merge_stars.py.
Same error.
I personally think the error message is fine. It says CUDA, it shows where the error is coming from and hiding the line where it is thrown will only make it harder to debug. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against b592e30 |
|
Thanks for dealing with the feedback!
Please let me know if you still wanted to do this, or if I should go ahead and see if I can implement some tests for this?
Yeah, as long as it is failing hard inside the cuda backends it is fine, just wanted to make sure it doesn't lead to weird importerrors. |
|
I tested on our HPC login node and it seems to work just fine. I just wanted to make sure I didn't make some stupid mistake. One could maybe test that voltools and cupy are not loaded as modules when calling merge_stars and extract_particles? |
|
Hey @MoritzWM, Thanks for checking! I am trying to design the tests. My idea is to mock out GPU availability somehow (probably setting I am getting somewhere, but not quite there yet. Will push to your branch once I have figured out a test that fails on main, but works on your branch. It is surprisingly hard to pretend to run on a machine without gpus even if the machine has them for one test and then have the GPU enabled for other tests. (Specifically struggling with python's import caching atm)😅. |
Hello,
I'm using pytom-match-pick on a HPC cluster and love it. Though one issue I'm having is that I sometimes play with the false positive rate or cutoff and I don't want to submit that as GPU-bound jobs. The same applies to merging star files.
I figured I could just move around some imports to make it not fail when there's no GPU. It still requires Cuda for installation though.