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

aiida-core installed by pip and bump python version 3.10 #316

Closed
wants to merge 3 commits into from

Conversation

unkcpz
Copy link
Member

@unkcpz unkcpz commented Oct 12, 2022

fixes #265

I am more kin to using pip to install aiida-core which is more well-tested and has fewer restrictions when comes to conda dependencies (for example if we want to have arm64 supported).
The current blocker of using python3.10 is that the wrapt 1.11 does not have python3.10 support which addressed by aiidateam/aiida-core#5698. It also shows that conda install of aiida-core is not well tested.

@unkcpz unkcpz changed the title aiida-core installed by pip and bump python version aiida-core installed by pip and bump python version 3.10 Oct 12, 2022
--file requirements.txt \
&& mamba clean --all -f -y && \
RUN pip install --no-cache-dir aiida-core==${AIIDA_VERSION} \
-r requirements.txt && \
Copy link
Contributor

Choose a reason for hiding this comment

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

This also means that things in requirement.txt will be installed by pip. Is that what you wanted?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right. To be honest, I have no idea why this requirements.txt is needed, it has aiida-core and pip included, but aiida-core is already installed with specifying the version.

Copy link
Contributor

@danielhollas danielhollas Oct 13, 2022

Choose a reason for hiding this comment

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

It is used below as a constraint file for conda and pip. But it is indeed strange that aiida-core version is specified in two places. This has already been noted in #311. Perhaps requirements.txt needs to be generated dynamically in the Dockerfile?

cc @csadorf

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, the requirements.txt file serves a double function as constraints file. The aiida-core version is not pinned using this file, but only constraint, as you can see here:


It would need to be updated if and only if the major version is changed.

The only other requirement in that file is the pinned pip version.

@unkcpz
Copy link
Member Author

unkcpz commented Oct 13, 2022

There is a big change here. The new base jupyter/minimal-notebook start to support linux/arm64/v8 which is not for the old base image. This is also the wired reason that the arm64 build pass and what is being downloaded in my arm64 machine is actually amd64.
I propose to remove the support to arm64 and add it back after I also start and test by using the self-hosted CI runner.

@unkcpz unkcpz marked this pull request as draft October 13, 2022 17:06
@unkcpz
Copy link
Member Author

unkcpz commented Oct 13, 2022

I'll revert this back using only mamba, I think the dependent issue needs to be fixed from aiida-core side.

We encounter not just one problem install aiida/aiidaservices from conda conda-forge/aiida-core-feedstock#76 and python3.10 support here.

I propose using pip for all the python packages and keeping mamba for the services and for installing the compiled software (e.g. QuantumESPRESS ..).

Any thoughts? @danielhollas @yakutovicha

@danielhollas
Copy link
Contributor

@unkcpz I guess I don't see a reason why not use pip to install AiiDA, if that is a better supported installation method, and I guess it makes sense given that dependencies of AiiDAlab apps are installed via pip. I wonder if that would on the other hand solve the issue that I reported in #320 with pymatgen.

I do worry a bit about the unintended consequences of this, as pip and conda sometimes interact strongly, but I don't have a deep experience with this. Definitely needs testing.

@csadorf
Copy link
Member

csadorf commented Oct 26, 2022

@unkcpz I guess I don't see a reason why not use pip to install AiiDA, if that is a better supported installation method, and I guess it makes sense given that dependencies of AiiDAlab apps are installed via pip. I wonder if that would on the other hand solve the issue that I reported in #320 with pymatgen.

I do worry a bit about the unintended consequences of this, as pip and conda sometimes interact strongly, but I don't have a deep experience with this. Definitely needs testing.

Installing aiida-core with pip means that all of its (long list of) dependencies are also installed via pip which means that the conda environment is basically converted into a pip-managed environment rendering it almost unusable for further conda installations.

@unkcpz
Copy link
Member Author

unkcpz commented Oct 26, 2022

Installing aiida-core with pip means that all of its (long list of) dependencies are also installed via pip which means that the conda environment is basically converted into a pip-managed environment rendering it almost unusable for further conda installations.

I see, thanks! However, I still see no advantages of using conda over pip in installing aiida-core.
Since we encounter some issues with installing aiida-core by conda, and pip-manager as I know is more widely used in AiiDA community. I think we can stick to the strategy to use conda/mamba for the non-python tools such as postgreSQL, rabbitmq, QE and for creating virtual environments. Meanwhile, we stick to pip to install the python packages and AiiDAlab apps.

@unkcpz unkcpz force-pushed the main branch 4 times, most recently from 486feb1 to 0897d53 Compare July 10, 2023 23:18
@danielhollas
Copy link
Contributor

Superseded by #337

@danielhollas danielhollas deleted the fix/py-ver-and-pip branch July 18, 2023 18:34
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.

Support Python 3.11
3 participants