131 fhi aims unclear extraction of species #138
Conversation
ndaelman-hu
added
bug
| @@ -88,6 +88,30 @@ class x_fhi_aims_section_controlIn_basis_func(MSection): | |||
| - | |||
| ''') | |||
|
|
|||
| x_fhi_aims_controlIn_basis_func_gauss_l = Quantity( | |||
| type=np.dtype(np.int32), | |||
There was a problem hiding this comment.
You can use type=np.int32 here and in other quantities.
| sec_basis_func.x_fhi_aims_controlInOut_basis_func_gauss_l = val[i][0] | ||
| alpha = np.array(val[i][1]) / ureg.angstrom ** 2 | ||
| sec_basis_func.x_fhi_aims_controlInOut_basis_func_primitive_gauss_alpha = alpha | ||
| # From legacy versions we know that 'free-atom' or 'free-ion' are connected to 'occ' |
There was a problem hiding this comment.
I can't comment into these code-specific changes too much, as the quantities do not have any description (and you might not want to add anything).
But if these quantities are being parsed in other places, then I'll simply delete this comment and possibly add it into the description of the method parse_atom_type itself.
| @@ -84,6 +89,54 @@ def get_files(pattern: str, filepath: str, stripname: str = '', deep: bool = Tru | |||
| return filenames | |||
|
|
|||
|
|
|||
| def hash_section( | |||
There was a problem hiding this comment.
Ok, let's see if I understand this method (feel free to correct me if not).
You want to pass the basis_set section (like above for FHIaims) to a hash inside the basis_set section itself. Not sure how this will help for comparison across entries, I guess depending on the settings the hash will be assigned so that basis_sets with the same setting have the same hash?
I like it, but in its current status, is this a real utility function? I mean, are you planning on using this for other parsers? I guess so, just wanted you to confirm this.
There was a problem hiding this comment.
I also think that a wrapper for a util.hash for msections is too specific for a util function.
| elif key in ['cut_pot', 'radial_base']: | ||
| setattr(sec_basis_set, 'x_fhi_aims_controlIn_%s' % key, np.array( | ||
| val[0], dtype=float)) | ||
| else: | ||
| try: | ||
| setattr(sec_basis_set, 'x_fhi_aims_controlIn_%s' % key, val[0]) | ||
| setattr(sec_basis_set, 'x_fhi_aims_controlIn_%s' % key, v[0]) |
There was a problem hiding this comment.
sec_basis_set.m_set(sec_basis_set.m_get_quantity_definition(f'x_fhi_aims_controlIn_{key}'), v[0])
| try: | ||
| evaluation_settings = zip(sections, subsections) | ||
| except Exception: # TODO: specify exception | ||
| raise ValueError( |
| `sections`: sections to be hashed together | ||
| `subsections`: list of bools, indicating whether to include subsections. Must be of same length as basis_settings. | ||
| `mode`: str, either `include` or `exclude` (default) | ||
| `quantities`: list of str, quantities to be included or excluded |
There was a problem hiding this comment.
would it not be better if instead of mode and quantities, you have (in)exclude: list of quantities to (in)exclude
| `mode`: str, either `include` or `exclude` (default) | ||
| `quantities`: list of str, quantities to be included or excluded | ||
| ''' | ||
| sections = [sections] if isinstance(sections, MSection) else sections |
There was a problem hiding this comment.
sections = [sections] if not isinstance(section, list) else sections
This is an intermediate implementation. It removes
controlInOutforcontrolIn, which has been patched.It also introduces general functions for producing tier hashes.
The latter still have to be applied, but this requires an update of the tier references.
Atm, this was done in an external project, but I can make it run locally. I just need to merge this first.
Maybe we can already merge this and add the rest later? Depends on how much time I get to work on this.
Closes #131 and https://gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR/-/issues/1576