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

Installing on Windows with defaults channel enabled can be a licensing problem #433

Open
nialov opened this issue Oct 8, 2024 · 5 comments · Fixed by #434
Open

Installing on Windows with defaults channel enabled can be a licensing problem #433

nialov opened this issue Oct 8, 2024 · 5 comments · Fixed by #434
Labels
question Further information is requested

Comments

@nialov
Copy link
Collaborator

nialov commented Oct 8, 2024

Anaconda has a license that makes using defaults channel problematic for organizations above 200 people (https://www.anaconda.com/blog/is-conda-free#summarize). I will make a PR removing the guidance to use the defaults channel. This will mean we cannot provide guidance on installing on Windows using conda with defaults channel as there is a high risk for users to breach the license unwittingly.

This is not a problem for Linux as all packages are available on conda-forge. However, for Windows, tensorflow is not maintained in conda-forge.

This removes the only tested installation method for Windows for eis_toolkit...

@nialov
Copy link
Collaborator Author

nialov commented Oct 8, 2024

@nmaarnio important for you to know.

@nmaarnio
Copy link
Collaborator

nmaarnio commented Oct 9, 2024

Good that you noticed this license issue, I was not aware of it.

tensorflow is currently used only for the MLP models which are available also in sklearn. There were some reasons why we decided to include tensorflow but I cannot recall them now. @msmiyels , do you remember and what do you think about changing our MLP implementations to use sklearn instead of tensorflow? Can the BNNs you are working on be implemented with sklearn?

When it comes to tested installation methods on Windows, so far EIS testers in our workshops have used pip and installed EIS Toolkit into a venv. We found out some Python versions that made the installation run without issues.

I'll go approve and merge the PR, but I'd be interested to know if we could manage without the packages that are not available in conda-forge.

nmaarnio added a commit that referenced this issue Oct 9, 2024
Remove defaults channel from environment.yml (Fixes #433)
@nmaarnio nmaarnio reopened this Oct 9, 2024
@nialov
Copy link
Collaborator Author

nialov commented Oct 10, 2024

When it comes to tested installation methods on Windows, so far EIS testers in our workshops have used pip and installed EIS Toolkit into a venv. We found out some Python versions that made the installation run without issues.

Yes, I remember you using this alternative method. I have been the one advocating for conda 😬 thinking that it would be simpler. But as long as we are using tensorflow, conda provides no good/simpler ways of installing on Windows.

If it turns out we can drop tensorflow from the dependencies, let me know and I will check that there are no other missing Windows packages and that I can get the environment working, Only then should eis_toolkit code be refactored so no one wastes time.

@msmiyels
Copy link
Collaborator

Hi @nmaarnio, @nialov, please excuse the late reply, I've been on leave for some days.

To the issue

  1. Oha 😲 , thank you for figuring this out 🙏
  2. Without having taken a look at the most recent version of the EIS-MLP, I think it would be possible to change it/use the sk-learn functionalities instead of tensorflow, which however, comes with some downsides or rather simplifications to it in regard to flexibility and model building (which is why we have choosen tensorflow over sk-learn in this particular case). Guess it would be more a complete replacement rather than a modification, but if really necessary, we must think about it 🙄
  3. The BNN, however, requires both tensorflow and tensorflow probability (came across some issues by comparing results and trying to fix this atm @nmaarnio)

The TF versions we're currently using for the BNN are tensorflow==2.13.1 and tensorflow_probability==0.21.0. Both versions are depending on each other, so there is only few degrees of freedom in terms of choosing a specific version (see here). In terms of compatibility, and if we can keep tensorflow in general and must keep the specific version, I need to check if the code runs with lower/other versions of tensorflow_probability as well.

I understood that this is only an issue if the packages come from the default repo, so would it be worth to try it with pip instead ❓ Can be accomplished by adding it to the .yml like

dependencies:
 - numpy==x.xx.xx
 - ...
 - pip:
    - tensorflow==x.xx.x
    - tensorflow_probability==x.xx.x
    - ...

@nialov
Copy link
Collaborator Author

nialov commented Oct 15, 2024

I understood that this is only an issue if the packages come from the default repo, so would it be worth to try it with pip instead ❓ Can be accomplished by adding it to the .yml like

dependencies:
 - numpy==x.xx.xx
 - ...
 - pip:
    - tensorflow==x.xx.x
    - tensorflow_probability==x.xx.x
    - ...

This is possible for the environment.yaml file in eis_toolkit but not in the package definition for conda-forge. Would be difficult to keep consistent, test, etc. ☹️

@msmiyels msmiyels added the question Further information is requested label Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants