aiidalab / aiidalab-widgets-base

Reusable widgets for AiiDAlab applications
MIT License
6 stars 17 forks source link

Do not create cell for 0D input structure #521

Open unkcpz opened 9 months ago

unkcpz commented 9 months ago

_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[bot] commented 9 months ago

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 ```diff @@ 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](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/521/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | Coverage Δ | | |---|---|---| | [python-3.10](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/521/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | `79.98% <90.90%> (+0.58%)` | :arrow_up: | | [python-3.8](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/521/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | `80.02% <90.90%> (+0.58%)` | :arrow_up: | | [python-3.9](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/521/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | `80.02% <90.90%> (+0.58%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/521?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | Coverage Δ | | |---|---|---| | [tests/test\_structures.py](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/521?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#diff-dGVzdHMvdGVzdF9zdHJ1Y3R1cmVzLnB5) | `100.00% <100.00%> (ø)` | | | [aiidalab\_widgets\_base/structures.py](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/521?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#diff-YWlpZGFsYWJfd2lkZ2V0c19iYXNlL3N0cnVjdHVyZXMucHk=) | `77.93% <84.61%> (+3.04%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

unkcpz commented 9 months ago

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: