LobsterPy icon indicating copy to clipboard operation
LobsterPy copied to clipboard

Published 20 hours ago • verified publisher icon JaGeo

Structure graphs

Open naik-aakash opened this issue 2 years ago • 4 comments

Hi Janine,

  • This is initial draft for getting structure_graphs .
  • I did add environment information from lobsterpy to graph nodes

Pending

  • [x] Add test cases
  • [x] Add bonding-antibonding percentages for COHP

Also provide feedback on the way it is implemented.

naik-aakash avatar Aug 24 '22 13:08 naik-aakash

Thanks. Will check it out. And, yes, pymatgen could be changed.

JaGeo avatar Aug 24 '22 13:08 JaGeo

Added the bonding-antibonding percentages as edge properties. The integral values obtained, however, don't match very well.

So I still need to recheck. If you spot any error when reviewing, I would be happy to rectify it.

naik-aakash avatar Aug 24 '22 15:08 naik-aakash

Added test case for graph module . Let me know if any further changes are needed. Tests failed because current pymatgen release does not have bond_label key in sg object

naik-aakash avatar Aug 26 '22 13:08 naik-aakash

Thanks @naik-aakash . I think it looks qvery good. Could you maybe make the suggested changes? I think, I can then merge it.

JaGeo avatar Nov 01 '22 14:11 JaGeo

Hi @JaGeo, let me know if anything more needs to be added or modified here. 😄

naik-aakash avatar Apr 17 '23 15:04 naik-aakash

Hi @naik-aakash , how does the test coverage currently look like and how diverse were the structures you tested the code on so far?

JaGeo avatar Apr 17 '23 16:04 JaGeo

Hi @naik-aakash , how does the test coverage currently look like and how diverse were the structures you tested the code on so far?

Hi @JaGeo , test coverage seems ~96 % for graphs.py module at the moment. I will test more different structures tomorrow and let you know if any issues pops up.

naik-aakash avatar May 22 '23 15:05 naik-aakash

Todo

  • [x] Test diverse structure types

naik-aakash avatar Jul 25 '23 16:07 naik-aakash

Hi @JaGeo, this PR could also be reviewed and merged

naik-aakash avatar Aug 25 '23 15:08 naik-aakash

@JaGeo, I have made the suggested changes.

In the following days, I will try to make changes in pymatgen to switch to any icoxxlist as a base file for neighbor analysis and add additional data from the two types of files. Then I will raise another PR to improve the current implementation 😄

Let me know if anything more needs to be done before it can be merged :-)

naik-aakash avatar Sep 13 '23 07:09 naik-aakash

Hi @JaGeo , here as well issues seem to be resolved so this could be merged

naik-aakash avatar Sep 14 '23 06:09 naik-aakash

There are still conflicts, unfortunately :(

Just resolved it now , should be fine. test files xD

naik-aakash avatar Sep 14 '23 06:09 naik-aakash