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

Support QE conda env in /opt/conda #784

Merged
merged 6 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions before-notebook.d/70_prepare-qe-executable.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,9 @@
# Debugging.
set -x

# Copy quantum espresso env to user space.
mkdir -p /home/${NB_USER}/.conda/envs
if [ ! -d /home/${NB_USER}/.conda/envs/quantum-espresso-${QE_VERSION} ]; then
ln -s /opt/conda/envs/quantum-espresso-${QE_VERSION} /home/${NB_USER}/.conda/envs/quantum-espresso-${QE_VERSION}

# Install qe so the progress bar not shown in the notebook when first time using app.
echo "Installing qe."
if [[ $(verdi code list -Y localhost -r | wc -l) -eq 0 ]]; then
echo "Installing QE codes..."
python -m aiidalab_qe install-qe
else
echo "Quantum ESPRESSO app is already installed."
echo "Quantum ESPRESSO codes are already installed."
fi
35 changes: 21 additions & 14 deletions src/aiidalab_qe/common/setup_codes.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# -*- coding: utf-8 -*-
from pathlib import Path
from shutil import which
from subprocess import CalledProcessError, run
Expand All @@ -21,9 +20,15 @@

QE_VERSION = "7.2"

CONDA_ENV_PREFIX = Path.home().joinpath(
".conda", "envs", f"quantum-espresso-{QE_VERSION}"
)

def get_qe_env():
# QE is already pre-installed in the QE image
path = Path(f"/opt/conda/envs/quantum-espresso-{QE_VERSION}")
if path.exists():
return path
else:
return Path.home().joinpath(".conda", "envs", f"quantum-espresso-{QE_VERSION}")

Comment on lines +24 to +30
Copy link
Member

Choose a reason for hiding this comment

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

This can cause an edge problem that when the container is started with qeapp image and then shifted to using full-stack image. The QE not able to be accessed.
We were trying to provide full-stack, qe-app, base-with-server images in demo server. But as suggested by @giovannipizzi, we agree on only keep qe-app image. This change then wont be a problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. But hopefully that will not happen in practice, and the solution is simply to re-install the app. By the way, in this case the current solution with the symlink would not work anyway since the symlink would be broken, right?

Copy link
Member

Choose a reason for hiding this comment

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

in this case the current solution with the symlink would not work anyway since the symlink would be broken, right?

I think it won't work.


# Add all QE codes with the calcjob entry point in the aiida-quantumespresso.
CODE_NAMES = (
Expand All @@ -46,7 +51,7 @@


def qe_installed():
return CONDA_ENV_PREFIX.exists()
return get_qe_env().exists()


def install_qe():
Expand All @@ -59,7 +64,7 @@ def install_qe():
"--channel",
"conda-forge",
"--prefix",
str(CONDA_ENV_PREFIX),
str(get_qe_env()),
f"qe={QE_VERSION}",
],
capture_output=True,
Expand Down Expand Up @@ -102,9 +107,9 @@ def _generate_string_to_setup_code(code_name, computer_name="localhost"):
except NotExistent:
label = f"{code_name}-{QE_VERSION}"
description = f"{code_name}.x ({QE_VERSION}) setup by AiiDAlab."
filepath_executable = str(CONDA_ENV_PREFIX.joinpath("bin", f"{code_name}.x"))
filepath_executable = get_qe_env().joinpath("bin", f"{code_name}.x")
default_calc_job_plugin = f"quantumespresso.{code_name}"
prepend_text = f'eval "$(conda shell.posix hook)"\\nconda activate {CONDA_ENV_PREFIX}\\nexport OMP_NUM_THREADS=1'
prepend_text = f'eval "$(conda shell.posix hook)"\\nconda activate {get_qe_env()}\\nexport OMP_NUM_THREADS=1'
python_code = """
computer = load_computer('{}')
code = InstalledCode(computer=computer,
Expand All @@ -116,7 +121,7 @@ def _generate_string_to_setup_code(code_name, computer_name="localhost"):
)

code.store()
""".format(
""".format( # noqa: UP032
computer_name,
label,
description,
Expand All @@ -134,7 +139,7 @@ def setup_codes():
try:
run(["python", "-c", python_code], capture_output=True, check=True)
except CalledProcessError as error:
raise RuntimeError(f"Failed to setup codes: {error}")
raise RuntimeError(f"Failed to setup codes: {error}") from None

Copy link
Contributor Author

@danielhollas danielhollas Jul 22, 2024

Choose a reason for hiding this comment

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

This is just to make stack traces nicer. Here's the correspoding flake8-bugbear rule as explained by ruff

❯ ruff rule B904
# raise-without-from-inside-except (B904)

Derived from the **flake8-bugbear** linter.

## What it does
Checks for `raise` statements in exception handlers that lack a `from`
clause.

## Why is this bad?
In Python, `raise` can be used with or without an exception from which the
current exception is derived. This is known as exception chaining. When
printing the stack trace, chained exceptions are displayed in such a way
so as make it easier to trace the exception back to its root cause.

When raising an exception from within an `except` clause, always include a
`from` clause to facilitate exception chaining. If the exception is not
chained, it will be difficult to trace the exception back to its root cause.

Example

try:
    ...
except FileNotFoundError:
    if ...:
        raise RuntimeError("...")
    else:
        raise UserWarning("...")

Use instead:

try:
    ...
except FileNotFoundError as exc:
    if ...:
        raise RuntimeError("...") from None
    else:
        raise UserWarning("...") from exc

References

Copy link
Member

Choose a reason for hiding this comment

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

Just curious, how you find these good thing? Is it from your LSP?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is from ruff, which I have setup as LSP in neovim.

While the pre-commit in this repo uses ruff, it doesn't have any ruff-specific configuration. I think we should copy over the configuration from AWB. I think we have a good experience with that set and it will make things consistent across the two repos.


def install(force=False):
Expand Down Expand Up @@ -175,7 +180,9 @@ def install(force=False):
try:
install_qe()
except CalledProcessError as error:
raise RuntimeError(f"Failed to create conda environment: {error}")
raise RuntimeError(
f"Failed to create conda environment: {error}"
) from None

# After installing QE, we install the corresponding
# AiiDA codes:
Expand All @@ -189,7 +196,7 @@ def install(force=False):
yield "Setting up all codes..."
run(["python", "-c", python_code], capture_output=True, check=True)
except CalledProcessError as error:
raise RuntimeError(f"Failed to setup codes: {error}")
raise RuntimeError(f"Failed to setup codes: {error}") from None

except Timeout:
# Assume that the installation was triggered by a different process.
Expand All @@ -198,7 +205,7 @@ def install(force=False):
if not codes_are_setup():
raise RuntimeError(
"Installation process did not finish in the expected time."
)
) from None


class QESetupWidget(ipw.VBox):
Expand Down Expand Up @@ -311,7 +318,7 @@ def _toggle_error_view(self, change):
@traitlets.observe("busy")
@traitlets.observe("error")
@traitlets.observe("installed")
def _update(self, change):
def _update(self, _change):
with self.hold_trait_notifications():
if self.hide_by_default:
self.layout.visibility = (
Expand Down
Loading