tbepler / topaz

Pipeline for particle picking in cryo-electron microscopy images using convolutional neural networks trained from positive and unlabeled examples. Also featuring micrograph and tomogram denoising with DNNs.
GNU General Public License v3.0
171 stars 63 forks source link

Version string rejected by cryoSPARC #177

Closed Guillawme closed 5 months ago

Guillawme commented 1 year ago

Hello,

Topaz used from within cryoSPARC used to work fine, and I don't know exactly in which version cryoSPARC changed (my topaz installation of version 0.2.5 has not changed), but now in cryoSPARC 4.3.1 I get the following error while trying to run a topaz job:

Traceback (most recent call last):
  File "cryosparc_master/cryosparc_compute/run.py", line 95, in cryosparc_compute.run.main
  File "/home/cryosparcuser/cryosparc/cryosparc_worker/cryosparc_compute/jobs/topaz/run_topaz.py", line 111, in run_topaz_wrapper_train
    topaz_version = utils.get_topaz_version(topaz_exec_path)
  File "/home/cryosparcuser/cryosparc/cryosparc_worker/cryosparc_compute/jobs/topaz/topaz_utils.py", line 135, in get_topaz_version
    assert semver.VersionInfo.isvalid(topaz_version_for_validation), \
AssertionError: Cannot determine topaz version, command "/home/guillaume/opt/wrappers/topaz --version" did not produce valid output: ""

It seems that semver.VersionInfo.isvalid() returns False for any string that does not match x.y.z in which x, y and z are numbers. It rejected TOPAZ 0.2.5a and 0.2.5a, but not 0.2.5.

My current workaround is to use the following wrapper script to trick cryoSPARC's version check into accepting to run topaz (this has worked just fine):

#!/usr/bin/env bash

if [[ $@ == '--version' ]]; then
    echo '0.2.5'
    exit 0
fi

source /home/guillaume/opt/miniconda3/etc/profile.d/conda.sh
conda activate topaz-0.2.5

topaz $@

conda deactivate

But this is not ideal, so I will shortly submit a PR addressing this, to make topaz --version print only an x.y.z string.

tbepler commented 1 year ago

Thanks for reporting this and opening a PR. We'll take a look.

Guillawme commented 5 months ago

Hello,

Bumping this because more people have started running into this issue.

I proposed a very simple fix in #178, a single line change to emit a version string in the form of x.y.z. Until this is merged, people have to find and implement the workaround in a wrapper script, which is not very user-friendly.

Thank you in advance!