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.

Sign up for GitHub

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

PhBaseWorkChain: better handler for convergence #986

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

PhBaseWorkChain: better handler for convergence #986

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
84d6e5e
`PhBaseWorkChain`: better handler for convergence
bastonero Nov 23, 2023
b593565
Improve logic
bastonero Nov 27, 2023
2626779
Merge branch 'main' of https://github.com/aiidateam/aiida-quantumespr…
bastonero Nov 28, 2023
c554577
Improve `ph` protocols
bastonero Nov 28, 2023
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
37 changes: 32 additions & 5 deletions src/aiida_quantumespresso/workflows/ph/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ class PhBaseWorkChain(ProtocolMixin, BaseRestartWorkChain):

defaults = AttributeDict({
'delta_factor_max_seconds': 0.95,
'delta_factor_alpha_mix': 0.90,
'alpha_mix': 0.70,
'delta_factor_alpha_mix': 0.65,
bastonero marked this conversation as resolved.
Show resolved Hide resolved
'nmix_ph': 4,
'alpha_mix': 0.7,
})

@classmethod
Expand Down Expand Up @@ -257,13 +258,39 @@ def handle_out_of_walltime(self, node):
@process_handler(priority=410, exit_codes=PhCalculation.exit_codes.ERROR_CONVERGENCE_NOT_REACHED)
def handle_convergence_not_reached(self, node):
"""Handle `ERROR_CONVERGENCE_NOT_REACHED` exit code: decrease the mixing beta and restart."""
self.ctx.restart_calc = node
factor = self.defaults.delta_factor_alpha_mix

nmix_ph = self.ctx.inputs.parameters.get('INPUTPH', {}).get('nmix_ph', self.defaults.nmix_ph)

if nmix_ph < 20: # in all these cases, better trying to use more iterations in the mixing first
if nmix_ph < 8: # 8~20 is the recommended range on the Quantum ESPRESSO documentation
nmix_ph_new = 8
if nmix_ph >= 8:
nmix_ph_new = 20

self.ctx.inputs.parameters.setdefault('INPUTPH', {})['nmix_ph'] = nmix_ph_new

action = f'increased number of mixing iteration from {nmix_ph} to {nmix_ph_new} and restarting'
self.report_error_handled(node, action)

return ProcessHandlerReport(True)

# now try playing with the mixing parameters
Copy link
Member

Choose a reason for hiding this comment

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

In my runs I had immediately set alpha_mix to 0.4 in the overrides. I didn't gather any statistics, but I did get a noticable improvement in convergence. Something to consider?

Copy link
Member

Choose a reason for hiding this comment

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

One additional note on the line below this one (which was already there ^^): The user can also set alpha_mix instead of alpha_mix(1) and this would work fine. If we only look for alpha_mix(1) and then set it, I think we override the alpha_mix specified by the user.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the user have to specify alpha_mix(i) = X, and not simply alpha_mix = X

Copy link
Member

Choose a reason for hiding this comment

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

I've always just used alpha_mix:

mabercx@uan04:/pfs/lustrep3/scratch/project_465000106/mabercx/88/a0/a0e0-3391-4e6b-b4da-fc98886eaa4a> grep alpha_mix aiida.in
  alpha_mix =   4.0000000000d-01
mabercx@uan04:/pfs/lustrep3/scratch/project_465000106/mabercx/88/a0/a0e0-3391-4e6b-b4da-fc98886eaa4a> grep alpha_mix aiida.out | head -n 5
      thresh= 1.000E-02 alpha_mix =  0.400 |ddv_scf|^2 =  1.410E-08
      thresh= 1.187E-05 alpha_mix =  0.400 |ddv_scf|^2 =  3.525E-09
      thresh= 5.937E-06 alpha_mix =  0.400 |ddv_scf|^2 =  7.637E-11
      thresh= 8.739E-07 alpha_mix =  0.400 |ddv_scf|^2 =  2.221E-13
      thresh= 4.713E-08 alpha_mix =  0.400 |ddv_scf|^2 =  3.612E-14

alpha_mix = self.ctx.inputs.parameters.get('INPUTPH', {}).get('alpha_mix(1)', self.defaults.alpha_mix)
alpha_mix_new = alpha_mix * factor

self.ctx.restart_calc = node
self.ctx.inputs.parameters.setdefault('INPUTPH', {})['alpha_mix(1)'] = alpha_mix_new
if self.ctx.inputs.parameters.get('INPUTPH', {}).get('alpha_mix(20)', -1) != -1:
mbercx marked this conversation as resolved.
Show resolved Hide resolved
action = 'no more efficient strategies for convergence.'
self.report_error_handled(node, action)
return ProcessHandlerReport(False)

if alpha_mix_new >= 0.1:
self.ctx.inputs.parameters.setdefault('INPUTPH', {})['alpha_mix(1)'] = alpha_mix_new
action = f'reduced alpha_mix from {alpha_mix} to {alpha_mix_new} and restarting'
else:
self.ctx.inputs.parameters.setdefault('INPUTPH', {})['alpha_mix(20)'] = 0.4
bastonero marked this conversation as resolved.
Show resolved Hide resolved
action = f'introduing alpha_mix(20)={alpha_mix_new} and restarting'
bastonero marked this conversation as resolved.
Show resolved Hide resolved

action = f'reduced alpha_mix from {alpha_mix} to {alpha_mix_new} and restarting'
self.report_error_handled(node, action)
return ProcessHandlerReport(True)
24 changes: 21 additions & 3 deletions tests/workflows/ph/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,33 @@ def test_handle_convergence_not_reached(generate_workchain_ph):
process.setup()
process.validate_parameters()

result = process.handle_convergence_not_reached(process.ctx.children[-1])
assert isinstance(result, ProcessHandlerReport)
assert result.do_break
assert process.ctx.inputs.parameters['INPUTPH']['nmix_ph'] == 8

result = process.handle_convergence_not_reached(process.ctx.children[-1])
assert isinstance(result, ProcessHandlerReport)
assert result.do_break
assert process.ctx.inputs.parameters['INPUTPH']['nmix_ph'] == 20

alpha_new = PhBaseWorkChain.defaults.alpha_mix * PhBaseWorkChain.defaults.delta_factor_alpha_mix

for _ in range(4):
result = process.handle_convergence_not_reached(process.ctx.children[-1])
assert isinstance(result, ProcessHandlerReport)
assert result.do_break
assert process.ctx.inputs.parameters['INPUTPH']['alpha_mix(1)'] == alpha_new
alpha_new = alpha_new * PhBaseWorkChain.defaults.delta_factor_alpha_mix

result = process.handle_convergence_not_reached(process.ctx.children[-1])
assert isinstance(result, ProcessHandlerReport)
assert result.do_break
assert process.ctx.inputs.parameters['INPUTPH']['alpha_mix(1)'] == alpha_new
assert process.ctx.inputs.parameters['INPUTPH']['alpha_mix(20)'] == 0.4

result = process.inspect_process()
assert result.status == 0
result = process.handle_convergence_not_reached(process.ctx.children[-1])
assert isinstance(result, ProcessHandlerReport)
assert not result.do_break


def test_handle_diagonalization_errors(generate_workchain_ph):
Expand Down
Loading