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

Adding then supercell estimator in the Settings Panel. #88

Merged
merged 5 commits into from
Oct 2, 2024

Conversation

mikibonacci
Copy link
Owner

@mikibonacci mikibonacci commented Sep 20, 2024

This address #34

Limitations:

  • use only the structure and the supercell size, not the other parameters (symprec, distinguish_kinds, is_symmetry)
  • to understand how we can adapt this to consider also the HubbardStructureData
  • performance is low (especially for the _reset_supercell reaction.
  • for low dim systems the reset just put 2 also in pbc[i] == False

Limitations:

- use only the structure and the supercell size, not the other parameters (symprec, distinguish_kinds, is_symmetry)
- to understand how we can adapt this to consider also the HubbardStructureData
- performance is low (especially for the `_reset_supercell` reaction.
- update the number of supercell only when all the three vectors are updated (from hint) or reset
- we provide also spinning loading icon when we compute the number of supercells.

- missing: symprec integration.
…uted

- adding a reset for symprec
- adding upper and lower bound 1 and 1e-7 (if out of this range, automatically reset to max or min)
- added tests for all the new widgets/logics
@mikibonacci
Copy link
Owner Author

Hi @AndresOrtegaGuerrero, I think the PR is ready. for now, we don't consider hubbard in the estimation of the supercells to be simulated, neither the dielectric workchain (i.e. how many finite Electric field calculations).

I added also symprec (merged from main) in the estimation logic, and also a spinning circle while we wait for the number of supercells to be determined. Please try it with SiO2 or gold, to see it in action for some seconds.

Thanks!


return

def _reset_supercell(self, _=None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

does it work with _reset_supercell(self): ?

def _reset_supercell(self, _=None):
if self.input_structure is not None:
reset_supercell = []
self.block = True
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add some dockstirng about this variable self.block

if self.input_structure is not None:
reset_supercell = []
self.block = True
for direction, periodic in zip(
Copy link
Collaborator

Choose a reason for hiding this comment

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

or direction, periodic in zip([self._sc_x, self._sc_y, self._sc_z], self.input_structure.pbc):
reset_supercell.append(2 if periodic else 1) , , what about this ? or is not that clear the way i suggest?

if self.block:
return

if self.symmetry_symprec.value > 1:
Copy link
Collaborator

Choose a reason for hiding this comment

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

self.symmetry_symprec.value = max(1e-5, min(self.symmetry_symprec.value, 1)) ,
one question , this is enforcing symprec to always be 1e-5 , what if the user sets a symprec like 1e-3 ?

@mikibonacci mikibonacci merged commit 89c70a2 into main Oct 2, 2024
2 checks passed
@mikibonacci mikibonacci deleted the fix/number_of_supercells branch October 2, 2024 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants