From 6669af580973603ff82ba8d60f0e02c105d0d885 Mon Sep 17 00:00:00 2001 From: Zhongwei Lu <115423652+GaryLZW@users.noreply.github.com> Date: Thu, 15 Aug 2024 11:37:27 +0100 Subject: [PATCH 1/9] Update counterpoise_onepot.py Force the function to use Cartesian coordinates --- carmm/analyse/counterpoise_onepot.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/carmm/analyse/counterpoise_onepot.py b/carmm/analyse/counterpoise_onepot.py index 0e5a94c0..21b4d66f 100644 --- a/carmm/analyse/counterpoise_onepot.py +++ b/carmm/analyse/counterpoise_onepot.py @@ -56,7 +56,11 @@ def counterpoise_calc(complex_struc, a_id, b_id, fhi_calc=None, a_name=None, b_n # Empty sites does not work with forces. Remove compute_forces. if 'compute_forces' in fhi_calc.parameters: - fhi_calc.parameters.pop('compute_forces') + if not ase_env_check('3.23.0'): + fhi_calc.set(compute_forces=False) + else: + fhi_calc.parameters['compute_forces']=False + fhi_calc.template.update_parameters(fhi_calc.implemented_properties, fhi_calc.parameters) # Create an empty list to store energies for postprocessing. energies = [] for index in range(4): @@ -72,10 +76,8 @@ def counterpoise_calc(complex_struc, a_id, b_id, fhi_calc=None, a_name=None, b_n energies.append(energy_i) else: fhi_calc.template.outputname = species_list[index] + '.out' - properties = fhi_calc.implemented_properties - parameters = fhi_calc.parameters - parameters['ghosts'] = ghosts_lists_cp[index] - fhi_calc.template.update_parameters(properties, parameters) + fhi_calc.parameters['ghosts'] = ghosts_lists_cp[index] + fhi_calc.template.update_parameters(fhi_calc.implemented_properties, fhi_calc.parameters) structures_cp[index].calc = fhi_calc if dry_run: structures_cp[index].calc.template.write_input(fhi_calc.profile, fhi_calc.directory, @@ -194,7 +196,8 @@ def calculate_energy_ghost_compatible(calc, atoms=None, properties=['energy'], from ase.calculators.calculator import Calculator import subprocess Calculator.calculate(calc, atoms, properties, system_changes) - calc.write_input(calc.atoms, properties, system_changes, ghosts=ghosts) + #Write inputfiles. Scaled positions does not with empty sites. + calc.write_input(calc.atoms, properties, system_changes, ghosts=ghosts, scaled=False) command = calc.command if dry_run: # Only for CI tests From 8094e57de9096e15770227e3272ed721581ed8aa Mon Sep 17 00:00:00 2001 From: Zhongwei Lu Date: Thu, 15 Aug 2024 12:02:23 +0100 Subject: [PATCH 2/9] Add a check with forces. Tidy up. --- carmm/analyse/counterpoise_onepot.py | 12 ++++++++---- examples/analyse_counterpoise.py | 4 ++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/carmm/analyse/counterpoise_onepot.py b/carmm/analyse/counterpoise_onepot.py index 21b4d66f..7f17f23b 100644 --- a/carmm/analyse/counterpoise_onepot.py +++ b/carmm/analyse/counterpoise_onepot.py @@ -59,8 +59,11 @@ def counterpoise_calc(complex_struc, a_id, b_id, fhi_calc=None, a_name=None, b_n if not ase_env_check('3.23.0'): fhi_calc.set(compute_forces=False) else: - fhi_calc.parameters['compute_forces']=False - fhi_calc.template.update_parameters(fhi_calc.implemented_properties, fhi_calc.parameters) + fhi_calc.parameters['compute_forces'] = False + if 'sc_accuracy_forces' in fhi_calc.parameters: + print('Stop calculation as there is a convergence criterion regarding force.', '\n', + 'Empty sites does not work with forces. Remove and check how it affects your results.') + return None # Create an empty list to store energies for postprocessing. energies = [] for index in range(4): @@ -77,7 +80,8 @@ def counterpoise_calc(complex_struc, a_id, b_id, fhi_calc=None, a_name=None, b_n else: fhi_calc.template.outputname = species_list[index] + '.out' fhi_calc.parameters['ghosts'] = ghosts_lists_cp[index] - fhi_calc.template.update_parameters(fhi_calc.implemented_properties, fhi_calc.parameters) + # Scaled positions does not with empty sites. + fhi_calc.parameters['scaled'] = False structures_cp[index].calc = fhi_calc if dry_run: structures_cp[index].calc.template.write_input(fhi_calc.profile, fhi_calc.directory, @@ -196,7 +200,7 @@ def calculate_energy_ghost_compatible(calc, atoms=None, properties=['energy'], from ase.calculators.calculator import Calculator import subprocess Calculator.calculate(calc, atoms, properties, system_changes) - #Write inputfiles. Scaled positions does not with empty sites. + # Write inputfiles. Scaled positions does not with empty sites. calc.write_input(calc.atoms, properties, system_changes, ghosts=ghosts, scaled=False) command = calc.command diff --git a/examples/analyse_counterpoise.py b/examples/analyse_counterpoise.py index 93faf352..3e6ce511 100644 --- a/examples/analyse_counterpoise.py +++ b/examples/analyse_counterpoise.py @@ -39,12 +39,16 @@ def test_analyse_counterpoise(): verbose=True, dry_run=True) cp_symbol = counterpoise_calc(CO, a_id=['C'], b_id=['O'], fhi_calc=toy_calc, dry_run=True) + toy_calc.parameters['sc_accuracy_forces'] = 0.0001 + cp_force_error = counterpoise_calc(CO, a_id=['C'], b_id=['O'], fhi_calc=toy_calc, dry_run=True) + # CP correction = A_only + B_only - A_plus_ghost - B_plus_ghost # This value should be added to the energy change of interest, such as adsorption energy. # CI-test assert cp_index == -8.707358006176946e-05 assert cp_symbol == -8.707358006176946e-05 + assert cp_force_error is None # Check the last created geometry.in file during the calculation. # These three lines below are only for CI-test purpose and should be deleted in actual calculation. From 7a643de2ed70ecb1a8812d5f7bbc292227b88e62 Mon Sep 17 00:00:00 2001 From: Zhongwei Lu Date: Thu, 15 Aug 2024 12:31:52 +0100 Subject: [PATCH 3/9] Tidy up. --- carmm/analyse/counterpoise_onepot.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/carmm/analyse/counterpoise_onepot.py b/carmm/analyse/counterpoise_onepot.py index 7f17f23b..da910b9d 100644 --- a/carmm/analyse/counterpoise_onepot.py +++ b/carmm/analyse/counterpoise_onepot.py @@ -56,10 +56,7 @@ def counterpoise_calc(complex_struc, a_id, b_id, fhi_calc=None, a_name=None, b_n # Empty sites does not work with forces. Remove compute_forces. if 'compute_forces' in fhi_calc.parameters: - if not ase_env_check('3.23.0'): - fhi_calc.set(compute_forces=False) - else: - fhi_calc.parameters['compute_forces'] = False + fhi_calc.parameters.pop('compute_forces') if 'sc_accuracy_forces' in fhi_calc.parameters: print('Stop calculation as there is a convergence criterion regarding force.', '\n', 'Empty sites does not work with forces. Remove and check how it affects your results.') From 0a082a2c667c0fdfd57b5992cb27a0fe915ed935 Mon Sep 17 00:00:00 2001 From: Zhongwei Lu Date: Thu, 15 Aug 2024 16:13:47 +0100 Subject: [PATCH 4/9] Edit docstrings --- carmm/analyse/counterpoise_onepot.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/carmm/analyse/counterpoise_onepot.py b/carmm/analyse/counterpoise_onepot.py index da910b9d..603cb3de 100644 --- a/carmm/analyse/counterpoise_onepot.py +++ b/carmm/analyse/counterpoise_onepot.py @@ -77,7 +77,7 @@ def counterpoise_calc(complex_struc, a_id, b_id, fhi_calc=None, a_name=None, b_n else: fhi_calc.template.outputname = species_list[index] + '.out' fhi_calc.parameters['ghosts'] = ghosts_lists_cp[index] - # Scaled positions does not with empty sites. + # Scaled positions does not work with empty sites. fhi_calc.parameters['scaled'] = False structures_cp[index].calc = fhi_calc if dry_run: @@ -197,7 +197,7 @@ def calculate_energy_ghost_compatible(calc, atoms=None, properties=['energy'], from ase.calculators.calculator import Calculator import subprocess Calculator.calculate(calc, atoms, properties, system_changes) - # Write inputfiles. Scaled positions does not with empty sites. + # Write inputfiles. Scaled positions does not work with empty sites. calc.write_input(calc.atoms, properties, system_changes, ghosts=ghosts, scaled=False) command = calc.command From 21785a0e57f28749e6a3ca491436418b7a25afc6 Mon Sep 17 00:00:00 2001 From: Zhongwei Lu Date: Thu, 15 Aug 2024 17:02:27 +0100 Subject: [PATCH 5/9] Raise KeyError when setting sc_accuracy_forces --- carmm/analyse/counterpoise_onepot.py | 7 ++++--- examples/analyse_counterpoise.py | 7 +++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/carmm/analyse/counterpoise_onepot.py b/carmm/analyse/counterpoise_onepot.py index 603cb3de..ed1619f0 100644 --- a/carmm/analyse/counterpoise_onepot.py +++ b/carmm/analyse/counterpoise_onepot.py @@ -58,9 +58,10 @@ def counterpoise_calc(complex_struc, a_id, b_id, fhi_calc=None, a_name=None, b_n if 'compute_forces' in fhi_calc.parameters: fhi_calc.parameters.pop('compute_forces') if 'sc_accuracy_forces' in fhi_calc.parameters: - print('Stop calculation as there is a convergence criterion regarding force.', '\n', - 'Empty sites does not work with forces. Remove and check how it affects your results.') - return None + if verbose: + print('Stop calculation as there is a convergence criterion regarding force.', '\n', + 'Empty sites does not work with forces. Remove and check how it affects your results.') + raise KeyError('Found sc_accuracy_forces in parameters! FHI-aims can not calculate forces on empty sites.') # Create an empty list to store energies for postprocessing. energies = [] for index in range(4): diff --git a/examples/analyse_counterpoise.py b/examples/analyse_counterpoise.py index 3e6ce511..cd30fefa 100644 --- a/examples/analyse_counterpoise.py +++ b/examples/analyse_counterpoise.py @@ -14,6 +14,7 @@ def test_analyse_counterpoise(): from carmm.analyse.counterpoise_onepot import counterpoise_calc from carmm.run.aims_calculator import get_aims_calculator from carmm.run.aims_path import set_aims_command + from unittest import TestCase # This is an example script for using counterpoise_calc for counterpoise (CP) correction. Please note the species # files in data/CO_BSSE are fake ones and default species settings are also deleted from aims.out. @@ -39,8 +40,11 @@ def test_analyse_counterpoise(): verbose=True, dry_run=True) cp_symbol = counterpoise_calc(CO, a_id=['C'], b_id=['O'], fhi_calc=toy_calc, dry_run=True) + # Test to make sure sc_accuracy_forces is not set, as FHI-aims can't calculate forces with empty sites toy_calc.parameters['sc_accuracy_forces'] = 0.0001 - cp_force_error = counterpoise_calc(CO, a_id=['C'], b_id=['O'], fhi_calc=toy_calc, dry_run=True) + forces_check = TestCase() + with forces_check.assertRaises(KeyError): + counterpoise_calc(CO, a_id=['C'], b_id=['O'], fhi_calc=toy_calc, dry_run=True) # CP correction = A_only + B_only - A_plus_ghost - B_plus_ghost # This value should be added to the energy change of interest, such as adsorption energy. @@ -48,7 +52,6 @@ def test_analyse_counterpoise(): # CI-test assert cp_index == -8.707358006176946e-05 assert cp_symbol == -8.707358006176946e-05 - assert cp_force_error is None # Check the last created geometry.in file during the calculation. # These three lines below are only for CI-test purpose and should be deleted in actual calculation. From e91c79941dc72702865bda027606dcc7de001038 Mon Sep 17 00:00:00 2001 From: Zhongwei Lu <115423652+GaryLZW@users.noreply.github.com> Date: Mon, 9 Sep 2024 11:52:46 +0100 Subject: [PATCH 6/9] fix bug in restart --- carmm/analyse/counterpoise_onepot.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/carmm/analyse/counterpoise_onepot.py b/carmm/analyse/counterpoise_onepot.py index ed1619f0..5d1c3495 100644 --- a/carmm/analyse/counterpoise_onepot.py +++ b/carmm/analyse/counterpoise_onepot.py @@ -196,7 +196,7 @@ def calculate_energy_ghost_compatible(calc, atoms=None, properties=['energy'], """ from ase.calculators.calculator import Calculator - import subprocess + import subprocess, os Calculator.calculate(calc, atoms, properties, system_changes) # Write inputfiles. Scaled positions does not work with empty sites. calc.write_input(calc.atoms, properties, system_changes, ghosts=ghosts, scaled=False) @@ -204,6 +204,9 @@ def calculate_energy_ghost_compatible(calc, atoms=None, properties=['energy'], if dry_run: # Only for CI tests command = '' # Used to be 'ls' + converged = False + if os.path.exists(calc.outfilename): + converged = calc.read_convergence() converged = calc.read_convergence() if (not converged) or dry_run: subprocess.check_call(command, shell=True, cwd=calc.directory) From 7d0abec2f37258bf7dabe7b8562a874ef8cfbe2f Mon Sep 17 00:00:00 2001 From: Zhongwei Lu <115423652+GaryLZW@users.noreply.github.com> Date: Mon, 9 Sep 2024 13:36:35 +0100 Subject: [PATCH 7/9] Update counterpoise_onepot.py --- carmm/analyse/counterpoise_onepot.py | 1 - 1 file changed, 1 deletion(-) diff --git a/carmm/analyse/counterpoise_onepot.py b/carmm/analyse/counterpoise_onepot.py index 5d1c3495..8216351e 100644 --- a/carmm/analyse/counterpoise_onepot.py +++ b/carmm/analyse/counterpoise_onepot.py @@ -207,7 +207,6 @@ def calculate_energy_ghost_compatible(calc, atoms=None, properties=['energy'], converged = False if os.path.exists(calc.outfilename): converged = calc.read_convergence() - converged = calc.read_convergence() if (not converged) or dry_run: subprocess.check_call(command, shell=True, cwd=calc.directory) From bf0b5923e94611136371c6bba500d3a7a48366fe Mon Sep 17 00:00:00 2001 From: Zhongwei Lu <115423652+GaryLZW@users.noreply.github.com> Date: Mon, 9 Sep 2024 13:49:17 +0100 Subject: [PATCH 8/9] Update analyse_counterpoise.py --- examples/analyse_counterpoise.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/analyse_counterpoise.py b/examples/analyse_counterpoise.py index cd30fefa..f089d7a6 100644 --- a/examples/analyse_counterpoise.py +++ b/examples/analyse_counterpoise.py @@ -44,7 +44,7 @@ def test_analyse_counterpoise(): toy_calc.parameters['sc_accuracy_forces'] = 0.0001 forces_check = TestCase() with forces_check.assertRaises(KeyError): - counterpoise_calc(CO, a_id=['C'], b_id=['O'], fhi_calc=toy_calc, dry_run=True) + counterpoise_calc(CO, a_id=['C'], b_id=['O'], fhi_calc=toy_calc, dry_run=True, verbose=True) # CP correction = A_only + B_only - A_plus_ghost - B_plus_ghost # This value should be added to the energy change of interest, such as adsorption energy. From 5d2dafa74d1e84d6a604c35d7a5c52c98dbfe11b Mon Sep 17 00:00:00 2001 From: Zhongwei Lu <115423652+GaryLZW@users.noreply.github.com> Date: Mon, 9 Sep 2024 13:54:04 +0100 Subject: [PATCH 9/9] Update counterpoise_onepot.py --- carmm/analyse/counterpoise_onepot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/carmm/analyse/counterpoise_onepot.py b/carmm/analyse/counterpoise_onepot.py index 8216351e..5f9fafd9 100644 --- a/carmm/analyse/counterpoise_onepot.py +++ b/carmm/analyse/counterpoise_onepot.py @@ -205,7 +205,7 @@ def calculate_energy_ghost_compatible(calc, atoms=None, properties=['energy'], if dry_run: # Only for CI tests command = '' # Used to be 'ls' converged = False - if os.path.exists(calc.outfilename): + if os.path.exists(calc.directory+'/'+calc.outfilename): converged = calc.read_convergence() if (not converged) or dry_run: subprocess.check_call(command, shell=True, cwd=calc.directory)