GispoCoding / eis_toolkit

Python library for mineral prospectivity mapping
https://eis-he.eu/
European Union Public License 1.2
19 stars 7 forks source link

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

Open nialov opened 1 week ago

nialov commented 1 week ago

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 commented 1 week ago

@nmaarnio important for you to know.

nmaarnio commented 1 week ago

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.

nialov commented 1 week ago

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 commented 4 days ago

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 commented 3 days ago

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. ☚ī¸