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

update nscf kpoints distance #446

Merged
merged 18 commits into from
Sep 26, 2023
Merged
Changes from 4 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
33 changes: 21 additions & 12 deletions src/aiidalab_qe/app/submission/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,15 @@ def submit(self, _=None):

def _get_qe_workchain_parameters(self) -> QeWorkChainParameters:
"""Get the parameters of the `QeWorkChain` from widgets."""
# Work chain settings
relax_type = self.workchain_settings.relax_type.value
electronic_type = self.workchain_settings.electronic_type.value
spin_type = self.workchain_settings.spin_type.value

run_bands = self.workchain_settings.bands_run.value
run_pdos = self.workchain_settings.pdos_run.value
protocol = self.workchain_settings.workchain_protocol.value

# create the the initial_magnetic_moments as None (Default)
initial_magnetic_moments = None
# create the override parameters for sub PwBaseWorkChain
Expand Down Expand Up @@ -449,9 +458,18 @@ def _get_qe_workchain_parameters(self) -> QeWorkChainParameters:

if key in ["base", "scf"]:
if self.advanced_settings.kpoints.override.value:
pw_overrides[key][
"kpoints_distance"
] = self.advanced_settings.kpoints.distance.value
kpoints_distance = self.advanced_settings.kpoints.distance.value
pw_overrides[key]["kpoints_distance"] = kpoints_distance

if protocol == "fast" and (
kpoints_distance < 0.5 and kpoints_distance > 0.15
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain the logic of how the nscf kpoints_distance is set? The basic idea seems to be that if the user-specified override for kpoints_distance is smaller than the corresponding protocol value for the nscf, the value is set to that of the override. But for fast there is this kpoints_distance > 0.15 condition. Why is this? If e.g. the user sets protocol to fast and overrides the kpoints_distance to 0.1 the nscf kpoints should not be adjusted?

A couple of other notes:

  1. Maybe instead of hard-coding the protocol values for the nscf, they can be extracted from the actual protocol? I thought we already did this elsewhere in the codebase. That way, the logic isn't broken when the protocol is changed in aiida-quantumespresso.
  2. I'm wondering if it makes sense to set the kpoints_distance for the nscf to the same one as the scf. Isn't the point of the nscf to do a calculation of the electronic states at fixed charge density for a denser mesh to improve the precision?
  3. Another option would be to also allow the user to specify the kpoints_distance override for the nscf, and e.g. the linear bands density as well. This will of course further complicate the interface, and maybe we don't want to expose all these inputs to the user.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mbercx I should modify this logic it should be
if protocol == "fast" and kpoints_distance < 0.5
My idea is that is kpoint_distance is set less to 0.5 (protocol) then nscf should be the same the user is setting. (or less but i dont know at what percentage we should decrease)

):
pw_overrides["nscf"]["kpoints_distance"] = kpoints_distance
if protocol == "moderate" and (kpoints_distance < 0.1):
pw_overrides["nscf"]["kpoints_distance"] = kpoints_distance
if protocol == "precise" and (kpoints_distance < 0.05):
pw_overrides["nscf"]["kpoints_distance"] = kpoints_distance

if (
self.advanced_settings.smearing.override.value
and self.workchain_settings.electronic_type.value == "metal"
Expand Down Expand Up @@ -480,15 +498,6 @@ def _get_qe_workchain_parameters(self) -> QeWorkChainParameters:
},
}

# Work chain settings
relax_type = self.workchain_settings.relax_type.value
electronic_type = self.workchain_settings.electronic_type.value
spin_type = self.workchain_settings.spin_type.value

run_bands = self.workchain_settings.bands_run.value
run_pdos = self.workchain_settings.pdos_run.value
protocol = self.workchain_settings.workchain_protocol.value

properties = []

if run_bands:
Expand Down