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

Do not create cell for 0D input structure #521

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

unkcpz
Copy link
Member

@unkcpz unkcpz commented Oct 3, 2023

_validate_and_fix_ase_cell function is not needed anymore, since it add a dummy cell to gas phase molecule. In the old AiiDA, there needed to be default cell, otherwise StructureData would not work.
But that has been fixed so we should get rid of this login in AWB.

The fix in aiida-core was done here aiidateam/aiida-core#5341 and released in version 2.0 https://github.com/aiidateam/aiida-core/blob/main/CHANGELOG.md#v200---2022-04-27

`_validate_and_fix_ase_cell` function is not needed anymore, since it add a
dummy cell to gas phase molecule. In the old AiiDA, there needed to be default
cell, otherwise StructureData would not work.
But that has been fixed so we should get rid of this login in AWB.

The fix in aiida-core was done here aiidateam/aiida-core#5341
and released in version 2.0 https://github.com/aiidateam/aiida-core/blob/main/CHANGELOG.md#v200---2022-04-27
@codecov
Copy link

codecov bot commented Oct 3, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (a1b5134) 79.39% compared to head (08b4a04) 79.98%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #521      +/-   ##
==========================================
+ Coverage   79.39%   79.98%   +0.58%     
==========================================
  Files          27       27              
  Lines        3849     3817      -32     
==========================================
- Hits         3056     3053       -3     
+ Misses        793      764      -29     
Flag Coverage Δ
python-3.10 79.98% <90.90%> (+0.58%) ⬆️
python-3.8 80.02% <90.90%> (+0.58%) ⬆️
python-3.9 80.02% <90.90%> (+0.58%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
tests/test_structures.py 100.00% <100.00%> (ø)
aiidalab_widgets_base/structures.py 77.93% <84.61%> (+3.04%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@unkcpz unkcpz marked this pull request as draft October 4, 2023 00:08
@unkcpz
Copy link
Member Author

unkcpz commented Oct 4, 2023

The test failed because the structure viewer is not fully ready for the molecule as we expected. When it used with storeable=True, the structure is supposed to saved as CifData, but it will cause exception as show below:

~/aiidalab-widgets-base/aiidalab_widgets_base/viewers.py in _valid_structure(self, change)
    834         if isinstance(structure, orm.Node):
    835             self.pk = structure.pk
--> 836             return structure.get_ase()
    837         raise TypeError(
    838             f"Unsupported type {type(structure)}, structure must be one of the following types: "

/opt/conda/lib/python3.9/site-packages/aiida/orm/nodes/data/cif.py in get_ase(self, **kwargs)
    406             return self.ase
    407         with self.open() as handle:
--> 408             return CifData.read_cif(handle, **kwargs)
    409 
    410     def set_ase(self, aseatoms):

/opt/conda/lib/python3.9/site-packages/aiida/orm/nodes/data/cif.py in read_cif(fileobj, index, **kwargs)
    314         # The read function returns a list as a cif file might contain multiple
    315         # structures.
--> 316         struct_list = read(fileobj, index=':', format='cif', **kwargs)
    317 
    318         if index is None:

/opt/conda/lib/python3.9/site-packages/ase/io/formats.py in read(filename, index, format, parallel, do_not_split_by_at_sign, **kwargs)
    731     io = get_ioformat(format)
    732     if isinstance(index, (slice, str)):
--> 733         return list(_iread(filename, index, format, io, parallel=parallel,
    734                            **kwargs))
    735     else:

/opt/conda/lib/python3.9/site-packages/ase/parallel.py in new_generator(*args, **kwargs)
    273             not kwargs.pop('parallel', True)):
    274             # Disable:
--> 275             for result in generator(*args, **kwargs):
    276                 yield result
    277             return

/opt/conda/lib/python3.9/site-packages/ase/io/formats.py in _iread(filename, index, format, io, parallel, full_output, **kwargs)
    801     # Make sure fd is closed in case loop doesn't finish:
    802     try:
--> 803         for dct in io.read(fd, *args, **kwargs):
    804             if not isinstance(dct, dict):
    805                 dct = {'atoms': dct}

/opt/conda/lib/python3.9/site-packages/ase/io/cif.py in read_cif(fileobj, index, store_tags, primitive_cell, subtrans_included, fractional_occupancies, reader)
    606             continue
    607 
--> 608         atoms = block.get_atoms(
    609             store_tags, primitive_cell,
    610             subtrans_included,

/opt/conda/lib/python3.9/site-packages/ase/io/cif.py in get_atoms(self, store_tags, primitive_cell, subtrans_included, fractional_occupancies)
    452                 'in the CIF file, i.e. when `subtrans_included` is True.')
    453 
--> 454         cell = self.get_cell()
    455         assert cell.rank in [0, 3]
    456 

/opt/conda/lib/python3.9/site-packages/ase/io/cif.py in get_cell(self)
    241         if cellpar is None:
    242             return Cell.new([0, 0, 0])
--> 243         return Cell.new(cellpar)
    244 
    245     def _raw_scaled_positions(self) -> Optional[np.ndarray]:

/opt/conda/lib/python3.9/site-packages/ase/cell.py in new(cls, cell)
     78         elif cell.shape == (6,):
     79             from ase.geometry.cell import cellpar_to_cell
---> 80             cell = cellpar_to_cell(cell)
     81         elif cell.shape != (3, 3):
     82             raise ValueError('Cell must be length 3 sequence, length 6 '

/opt/conda/lib/python3.9/site-packages/ase/geometry/cell.py in cellpar_to_cell(cellpar, ab_normal, a_direction)
    127     cy = (cos_alpha - cos_beta * cos_gamma) / sin_gamma
    128     cz_sqr = 1. - cx * cx - cy * cy
--> 129     assert cz_sqr >= 0
    130     cz = sqrt(cz_sqr)
    131     vc = c * np.array([cx, cy, cz])

AssertionError: 

unkcpz pushed a commit to unkcpz/aiidalab-widgets-base that referenced this pull request Nov 16, 2023
 Remove the code blocker when updating code widgets.
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.

1 participant