Change install to one env file & update docs #484

mathieuboudreau · 2021-03-22T18:29:36Z

Checklist

I've given this PR a concise, self-descriptive, and meaningful title
I've linked relevant issues in the PR body
I've applied the relevant labels to this PR
I've added relevant tests for my contribution
I've updated the documentation and/or added correct docstrings
I've assigned a reviewer
I've consulted ADS's internal developer documentation to ensure my contribution is in line with any relevant design decisions

Resolves #481, Resolves #482, Resolves #483

mathieuboudreau · 2021-03-22T18:32:16Z

@jcohenadad I had to leave activating the conda env in the installation step, as the installation step needs another command within in (pip install -e .). However, I've added a small section under the "Using AxonDeepSeg" section to say that you need to activate the venv if not already activated.

mariehbourget · 2021-03-22T20:58:48Z

We made the changes to use only the common environment.yml file to install ADS on all OS.
I tested it on my laptop in Ubuntu 20.04 in WSL1 and Windows 10, everything is working as expected.

@vs74, @Stoyan-I-A, @dyt811, @ahill187, @joshuacwnewton, @gisellemartel Could you re-test the installation using this branch? The rendered doc is here. Thank you everyone for your help!

vasudev-sharma · 2021-03-22T21:32:51Z

Installed ADS by referring to the docs of this PR. I had no issues with the installation on MacOS Mojave. Tested axondeepseg_test and fsleyes too; everything works like a charm.

joshuacwnewton · 2021-03-22T22:32:55Z

Installing worked for me on Ubuntu 20.04, as well. 🙂

Stoyan-I-A · 2021-03-23T00:53:29Z

The installation worked. Tested on Windows 10.

gisellemartel · 2021-03-23T02:00:26Z

Installation worked for me on MacOS Catalina 10.15.7! Testing the installation however failed for axondeepseg_test. Here is a log: log.txt

Stoyan-I-A · 2021-03-23T03:02:59Z

Installation worked for me on MacOS Catalina 10.15.7! Testing the installation however failed for axondeepseg_test. Here is a log: log.txt

Have you used the Theano backend in the past? It looks like the software is trying to use Theano instead of tensorflow.

mathieuboudreau · 2021-03-23T12:00:52Z

Installation worked for me on MacOS Catalina 10.15.7! Testing the installation however failed for axondeepseg_test. Here is a log: log.txt

Have you used the Theano backend in the past? It looks like the software is trying to use Theano instead of tensorflow.

Hmm interesting, could this be a tool that was installed with pip and the --user install flag? I've encountered instances where those have leaked into my venvs, very annoying and tough to troubleshoot if you're not aware/remember.

mariehbourget · 2021-03-23T14:37:15Z

I found this, it seems to be an issue when installing Keras through conda-forge, though the post is 3 years old.

The only way I found to switch the backend permanently was to explicitly export the variable in ${CONDA_PREFIX}/etc/conda/activate.d/keras_activate.sh with export KERAS_BACKEND="tensorflow"

mathieuboudreau · 2021-03-23T14:56:29Z

I found this, it seems to be an issue when installing Keras through conda-forge, though the post is 3 years old.

The only way I found to switch the backend permanently was to explicitly export the variable in ${CONDA_PREFIX}/etc/conda/activate.d/keras_activate.sh with export KERAS_BACKEND="tensorflow"

@gisellemartel could you try this fix temporarily to see if resolves the issue? If so, @mariehbourget let's just make a quick Notice for it on the ReadTheDocs, as it seems like a rare issue and we're migrating away from Keras when we move to IVADOMED soon.

mariehbourget · 2021-03-24T14:48:11Z

@gisellemartel, did you get the chance to test the fix describe above?

gisellemartel · 2021-03-24T16:23:33Z

@gisellemartel, did you get the chance to test the fix describe above?

Thank you @mariehbourget @mathieuboudreau @Stoyan-I-A for your suggestions. I tried the suggested fix and it worked! I do still get some warnings after running the test however
log.txt

mariehbourget · 2021-03-24T18:10:08Z

Thanks @gisellemartel for the tests.

@mathieuboudreau, I added the warning to the doc. Can you review it and let me know if that is ok with you (I can't assign you officially since you opened the PR). Thanks!

joshuacwnewton · 2021-03-24T18:13:55Z

docs/source/documentation.rst

+.. NOTE :: For some users, the test may fail because Keras is using Theano backend instead of Tensorflow. In that case, you will see the line ``Using Theano backend.`` when launching ``axondeepseg_test``. To fix this issue, add the line ``export KERAS_BACKEND="tensorflow"`` at the end of the ``<your_conda_install_location>\envs\<your_environment_name>/etc/conda/activate.d/keras_activate.sh`` file, then deactivate and reactivate your environment. The test should print ``Using Tensorflow backend.`` and pass.
+


Do we need to warn about this?

IIRC, the Keras project has moved on from non-Tensorflow backends ever since it was integrated into TF. They used to have this warning:

Multi-backend Keras has been discontinued. At this time, we recommend that Keras users who use multi-backend Keras with the TensorFlow backend switch to tf.keras in TensorFlow 2.0.

So, I would hope that typically Keras users wouldn't run into this issue.

(Though, now that I'm looking into this, they recently edited their README to remove information about this. Now it's "under construction". Hrm.)

We decided to warn about this because in particular, one of our test users encountered exactly this issue here in this thread =)

I guess what I'm asking is, could this be an isolated incident that doesn't apply to most users? 😛

mathieuboudreau · 2021-03-24T18:17:02Z

@mariehbourget The message reads well for me; if the tests pass you can approve and merge, which will open the door to merging #479.

mathieuboudreau · 2021-03-24T18:27:53Z

Haha fair. Maybe it coyld be in a troubleshooting section, but lets merge it here for now as the other PR is urgent Téléchargez Outlook pour Android<https://aka.ms/ghei36>

________________________________ From: Joshua Newton ***@***.***> Sent: Wednesday, March 24, 2021 3:50:19 PM To: neuropoly/axondeepseg ***@***.***> Cc: Mathieu Boudreau ***@***.***>; Mention ***@***.***> Subject: Re: [neuropoly/axondeepseg] Change install to one env file & update docs (#484) @joshuacwnewton commented on this pull request.

________________________________ In docs/source/documentation.rst<#484 (comment)>:

+.. NOTE :: For some users, the test may fail because Keras is using Theano backend instead of Tensorflow. In that case, you will see the line ``Using Theano backend.`` when launching ``axondeepseg_test``. To fix this issue, add the line ``export KERAS_BACKEND="tensorflow"`` at the end of the ``<your_conda_install_location>\envs\<your_environment_name>/etc/conda/activate.d/keras_activate.sh`` file, then deactivate and reactivate your environment. The test should print ``Using Tensorflow backend.`` and pass.

+ I guess what I'm saying is, could this be an isolated incident that doesn't apply to most users? 😛 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#484 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAK25ZOZNC2WPMSS4MGIT3TTFIUOHANCNFSM4ZTU5G2A>.

mathieuboudreau added 4 commits March 22, 2021 15:39

Delete environment_Linux.yml

6b16b53

Delete environment_Windows.yml

acb8df4

Delete environment_macOS.yml

ca09f7d

Update documentation.rst

f21d779

mathieuboudreau added documentation category: the change is documentation related to AxonDeepSeg installation category: the change alters the installation of AxonDeepSeg labels Mar 22, 2021

mathieuboudreau assigned mariehbourget Mar 22, 2021

Add missing tabs

44edd7f

mathieuboudreau and others added 4 commits March 22, 2021 16:03

Add missing syntax

a15fcfc

Update .travis.yml

22e3d53

Fix formatting doc

6019c4c

Fix formatting in doc

ee9b185

mariehbourget mentioned this pull request Mar 22, 2021

TravisCI to GitHub Action Migration #479

Merged

7 tasks

Merge branch 'master' into mb/48-environment

0d770cd

Add warning in doc for Theano vs Tensorflow

02081f1

joshuacwnewton reviewed Mar 24, 2021

View reviewed changes

mariehbourget approved these changes Mar 24, 2021

View reviewed changes

mariehbourget merged commit ffa7680 into master Mar 24, 2021

dyt811 mentioned this pull request Mar 25, 2021

add lib versioning MotionCorrect/ivadomed#1

Merged

7 tasks

mariehbourget mentioned this pull request Jun 28, 2021

slow conda dependency resolution #537

Open

mathieuboudreau deleted the mb/48-environment branch March 11, 2024 23:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change install to one env file & update docs #484

Change install to one env file & update docs #484

mathieuboudreau commented Mar 22, 2021 •

edited by mariehbourget

mathieuboudreau commented Mar 22, 2021

mariehbourget commented Mar 22, 2021

vasudev-sharma commented Mar 22, 2021 •

edited

joshuacwnewton commented Mar 22, 2021

Stoyan-I-A commented Mar 23, 2021

gisellemartel commented Mar 23, 2021 •

edited

Stoyan-I-A commented Mar 23, 2021

mathieuboudreau commented Mar 23, 2021 •

edited

mariehbourget commented Mar 23, 2021

mathieuboudreau commented Mar 23, 2021

mariehbourget commented Mar 24, 2021

gisellemartel commented Mar 24, 2021

mariehbourget commented Mar 24, 2021

joshuacwnewton Mar 24, 2021 •

edited

mathieuboudreau Mar 24, 2021

joshuacwnewton Mar 24, 2021 •

edited

mathieuboudreau commented Mar 24, 2021

mathieuboudreau commented Mar 24, 2021 via email

		.. NOTE :: For some users, the test may fail because Keras is using Theano backend instead of Tensorflow. In that case, you will see the line ``Using Theano backend.`` when launching ``axondeepseg_test``. To fix this issue, add the line ``export KERAS_BACKEND="tensorflow"`` at the end of the ``<your_conda_install_location>\envs\<your_environment_name>/etc/conda/activate.d/keras_activate.sh`` file, then deactivate and reactivate your environment. The test should print ``Using Tensorflow backend.`` and pass.

Change install to one env file & update docs #484

Change install to one env file & update docs #484

Conversation

mathieuboudreau commented Mar 22, 2021 • edited by mariehbourget

Checklist

mathieuboudreau commented Mar 22, 2021

mariehbourget commented Mar 22, 2021

vasudev-sharma commented Mar 22, 2021 • edited

joshuacwnewton commented Mar 22, 2021

Stoyan-I-A commented Mar 23, 2021

gisellemartel commented Mar 23, 2021 • edited

Stoyan-I-A commented Mar 23, 2021

mathieuboudreau commented Mar 23, 2021 • edited

mariehbourget commented Mar 23, 2021

mathieuboudreau commented Mar 23, 2021

mariehbourget commented Mar 24, 2021

gisellemartel commented Mar 24, 2021

mariehbourget commented Mar 24, 2021

joshuacwnewton Mar 24, 2021 • edited

Choose a reason for hiding this comment

mathieuboudreau Mar 24, 2021

Choose a reason for hiding this comment

joshuacwnewton Mar 24, 2021 • edited

Choose a reason for hiding this comment

mathieuboudreau commented Mar 24, 2021

mathieuboudreau commented Mar 24, 2021 via email

mathieuboudreau commented Mar 22, 2021 •

edited by mariehbourget

vasudev-sharma commented Mar 22, 2021 •

edited

gisellemartel commented Mar 23, 2021 •

edited

mathieuboudreau commented Mar 23, 2021 •

edited

joshuacwnewton Mar 24, 2021 •

edited

joshuacwnewton Mar 24, 2021 •

edited