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

Mh conda #137

Merged
merged 13 commits into from
Nov 28, 2022
Merged

Mh conda #137

merged 13 commits into from
Nov 28, 2022

Conversation

MohamadHarastani
Copy link
Collaborator

In this PR:

  • during the installation of ContinuousFlex, a conda environment is created that is clone of scipion3 conda environment.
  • ContinuousFlex no longer installs lapack and arpack locally, but on the conda environment.
  • on the new conda environment the deeplearning and optical flow packages are installed.
  • TomoFlow and DeepHEMNMA will use the new conda environment.

Needed to check:
Pablo: could you please review the changes on the init.py file, and see if I made any illegal thing?
Remi: can you please check if Genesis installs? It doesn't install on my computer unless I set (default=False) in the init file.
Ilyes: my modifications are minor on DeepHEMNMA. But can you check if we can sort out this warning when with the viewer of DeepHEMNMA train? Maybe --load_fast=false in there??

TensorFlow installation not found - running with reduced feature set.

NOTE: Using experimental fast data loading logic. To disable, pass
    "--load_fast=false" and report issues on GitHub. More details:
    https://github.com/tensorflow/tensorboard/issues/4784

Serving TensorBoard on localhost; to expose to the network, use a proxy or pass --bind_all
TensorBoard 2.8.0 at http://localhost:6006/ (Press CTRL+C to quit)

This PR closes #136 #127

@MohamadHarastani
Copy link
Collaborator Author

Sorry for the multiple commits before, I was testing on another computer and I kept correcting mistakes. The tests passed except 3 that Remi is the best to give us answer on issue #138
I am happy with how it is now until I get feedback from you!

@mms29
Copy link
Collaborator

mms29 commented Nov 23, 2022

I am running the installation :
conda create -y -n continuousflex-git --clone scipion3 && conda activate continuousflex-git && conda install -y -c conda-forge arpack lapack && touch env-created.txt

But for conda activate continuousflex-git it says :

CommandNotFoundError: Your shell has not been properly configured to use 'conda activate'.
To initialize your shell, run

    $ conda init <SHELL_NAME>

It looks like the scipion installation uses sh which conda does not support and not bash which is my default shell
I am trying to find a workaround to see how to force scipion to use my default shell
Did you force Scipion to use bash somewhere in the config file ?

@mms29
Copy link
Collaborator

mms29 commented Nov 24, 2022

It looks like my problem is that :
conda activate runs fine but :
scipion3 run "conda activate" give me the error.

@mms29
Copy link
Collaborator

mms29 commented Nov 24, 2022

Ok I solved my problem, the variable CONDA_ACTIVATION_CMD was not properly set in my scipion config file.
Everything works fine including GENESIS installation and tests

Copy link
Collaborator

@mms29 mms29 left a comment

Choose a reason for hiding this comment

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

Everything looks fine, the tests are running OK.
If I understand well, I have to add this "getcontinuousflexcmd" each time I call the program for genesis ?
If so, I can to the modifications on this branch 'mh_conda"

@pconesa
Copy link

pconesa commented Nov 24, 2022

I got a dependency conflict with another plugin scipion-em-cryosparc.

ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
scipion-em-cryosparc2 3.3.1 requires llvmlite<0.32.0, but you have llvmlite 0.39.1 which is incompatible.
scipion-em-cryosparc2 3.3.1 requires numba<=0.47.0, but you have numba 0.56.4 which is incompatible.

All this is triggered by umap dependency... lowering the version may solve the issue. Is this possible?

@MohamadHarastani
Copy link
Collaborator Author

MohamadHarastani commented Nov 24, 2022

I got a dependency conflict with another plugin scipion-em-cryosparc.

ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts. scipion-em-cryosparc2 3.3.1 requires llvmlite<0.32.0, but you have llvmlite 0.39.1 which is incompatible. scipion-em-cryosparc2 3.3.1 requires numba<=0.47.0, but you have numba 0.56.4 which is incompatible.

All this is triggered by umap dependency... lowering the version may solve the issue. Is this possible?

@pconesa : thanks for the feedback, I removed umap-learn from the requirements and added it to the conda env. Can you pull and test again?
@mms29 : I didn't test what happened in my last commit, can you test that? (umap for pdb dimred) 28a88eb

@mms29
Copy link
Collaborator

mms29 commented Nov 25, 2022

Your code for UMAP was good, there was just one space missing!
I also adapted genesis to work inside the new environment.
I just pushed the changes

@MohamadHarastani
Copy link
Collaborator Author

Thanks @mms29 for the check!
Before we merge this PR, can you take a look at the init.py?
There are three points:
1- Can we remove the dependency on cmake and install it using conda? https://anaconda.org/anaconda/cmake. This will sort out another problem!
2- Can you check with me what are the environment variables that are useless? I copied them from xmipp and they were useful for something at one moment, but maybe not anymore. If they are useless or can be replaced with something more meaningful, please go ahead.
3- Shall we transfer the test data definition to tests.init.py? It makes more sense in there.
We better clean up and make a new release.. the fixes have accumulated.
Cheers

@mms29
Copy link
Collaborator

mms29 commented Nov 27, 2022

  1. We can remove the check on cmake version, no need to install cmake with conda as none of our program use it now.
  2. CONTINUOUSFLEX_HOME was not referenced anywhere in the code, I assume it is not used. It does not make sense to set it at "xmipp", I 'd rather set it to the real continuousflex home (continuousflex.path[0]). For the other variable, it looks fine.
  3. Yes, I moved to tests/init.py, the tests run fine

@mms29
Copy link
Collaborator

mms29 commented Nov 27, 2022

I pushed some changes corresponding to these comments

@MohamadHarastani
Copy link
Collaborator Author

Thanks a lot Remi!
@ilyes-hm : Hey man! can you test and approve this PR? We can make a clean up on the devel branch and make a new release.
Cheers

@MohamadHarastani MohamadHarastani merged commit 2dbc88b into devel Nov 28, 2022
@MohamadHarastani MohamadHarastani deleted the mh_conda branch December 5, 2022 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants