Skip to content

Commit

Permalink
PhCalculation: Fix bug when only_initialization=True
Browse files Browse the repository at this point in the history
When the `only_initialization` setting is set to `True` the plugin would
add an instruction to the `remote_copy_list` to copy `DYN_MAT` folder
from the `parent_folder`. However, this would fail if the `parent_folder`
was that of a `PwCalculation`, which is one of the main use cases of the
`PhCalculation`.

It is not entirely clear why this was added in the commit that added it
dfdca7e which was just adding a plugin
for EPW. We assume it was added by mistake and so remove it.

A test is added for the `only_initialization=True` case since that
didn't exist yet. Unfortunately, this test won't prevent the same bug
being introduced because the incorrect code isn't triggered until the
engine tries to execute the copy commands. This is not possible to
reproduce in this simple unit test.
  • Loading branch information
sphuber committed Jun 2, 2022
1 parent 05d9f33 commit 0fe6f3d
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 39 deletions.
5 changes: 0 additions & 5 deletions src/aiida_quantumespresso/calculations/ph.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,11 +298,6 @@ def prepare_for_submission(self, folder):
with folder.open(f'{self._PREFIX}.EXIT', 'w') as handle:
handle.write('\n')

remote_copy_list.append((
parent_folder.computer.uuid,
os.path.join(parent_folder.get_remote_path(), self._FOLDER_DYNAMICAL_MATRIX), '.'
))

codeinfo = datastructures.CodeInfo()
codeinfo.cmdline_params = (list(settings.pop('CMDLINE', [])) + ['-in', self.metadata.options.input_filename])
codeinfo.stdout_name = self.metadata.options.output_filename
Expand Down
49 changes: 16 additions & 33 deletions tests/calculations/test_ph.py
Original file line number Diff line number Diff line change
@@ -1,34 +1,19 @@
# -*- coding: utf-8 -*-
"""Tests for the `PhCalculation` class."""
from pathlib import Path

from aiida import orm
from aiida.common import datastructures
from aiida.plugins import CalculationFactory

from aiida_quantumespresso.utils.resources import get_default_options

PwCalculation = CalculationFactory('quantumespresso.pw')
PhCalculation = CalculationFactory('quantumespresso.ph')


def test_ph_default(
fixture_localhost, fixture_sandbox, generate_calc_job, fixture_code, generate_kpoints_mesh, generate_remote_data,
file_regression
):
def test_ph_default(fixture_sandbox, generate_inputs_ph, generate_calc_job, file_regression):
"""Test a default `PhCalculation`."""
entry_point_name = 'quantumespresso.ph'
parent_entry_point = 'quantumespresso.pw'
remote_path = fixture_sandbox.abspath

inputs = {
'code': fixture_code(entry_point_name),
'parent_folder': generate_remote_data(fixture_localhost, remote_path, parent_entry_point),
'qpoints': generate_kpoints_mesh(2),
'parameters': orm.Dict({'INPUTPH': {}}),
'metadata': {
'options': get_default_options()
}
}

inputs = generate_inputs_ph()
calc_info = generate_calc_job(fixture_sandbox, entry_point_name, inputs)

cmdline_params = ['-in', 'aiida.in']
Expand All @@ -51,33 +36,31 @@ def test_ph_default(


def test_ph_qpoint_list(
fixture_localhost, fixture_sandbox, generate_calc_job, fixture_code, generate_structure, generate_kpoints_mesh,
generate_remote_data, file_regression
fixture_sandbox, generate_inputs_ph, generate_calc_job, generate_structure, generate_kpoints_mesh, file_regression
):
"""Test a `PhCalculation` with a qpoint list instead of a mesh."""
entry_point_name = 'quantumespresso.ph'
parent_entry_point = 'quantumespresso.pw'
remote_path = fixture_sandbox.abspath

structure = generate_structure()
kpoints = generate_kpoints_mesh(2).get_kpoints_mesh(print_list=True)
qpoints = orm.KpointsData()
qpoints.set_cell(structure.cell)
qpoints.set_kpoints(kpoints)

inputs = {
'code': fixture_code(entry_point_name),
'parent_folder': generate_remote_data(fixture_localhost, remote_path, parent_entry_point),
'qpoints': qpoints,
'parameters': orm.Dict({'INPUTPH': {}}),
'metadata': {
'options': get_default_options()
}
}

inputs = generate_inputs_ph()
inputs['qpoints'] = qpoints
generate_calc_job(fixture_sandbox, entry_point_name, inputs)

with fixture_sandbox.open('aiida.in') as handle:
input_written = handle.read()

file_regression.check(input_written, encoding='utf-8', extension='.in')


def test_ph_initialization_only(fixture_sandbox, generate_inputs_ph, generate_calc_job):
"""Test a ``PhCalculation`` that should just run the initialization."""
entry_point_name = 'quantumespresso.ph'
inputs = generate_inputs_ph()
inputs['settings'] = orm.Dict({'only_initialization': True})
generate_calc_job(fixture_sandbox, entry_point_name, inputs)
assert (Path(fixture_sandbox.abspath) / f'{PhCalculation._PREFIX}.EXIT').exists() # pylint: disable=protected-access
2 changes: 1 addition & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ def _generate_inputs_ph():
from aiida_quantumespresso.utils.resources import get_default_options

inputs = {
'code': fixture_code('quantumespresso.matdyn'),
'code': fixture_code('quantumespresso.ph'),
'parent_folder': generate_remote_data(fixture_localhost, fixture_sandbox.abspath, 'quantumespresso.pw'),
'qpoints': generate_kpoints_mesh(2),
'parameters': Dict({'INPUTPH': {}}),
Expand Down

0 comments on commit 0fe6f3d

Please sign in to comment.