TREX-CoE / trexio_tools

Set of tools for trexio files
BSD 3-Clause "New" or "Revised" License
18 stars 8 forks source link

Pyscf to trexio converter via `convert-from` #15

Closed q-posev closed 1 year ago

q-posev commented 1 year ago

Integration of @kousuke-nakano 's converter into the driver script via convert-from.

In the meantime I improved error handling for the non-supported cases by raising NotImplementedError.

@kousuke-nakano I have one question: I managed to convert two of your examples (from the data folder): water.chk and diamond_single_k.chk. However, for the multiple k points I get the following error:

  File "/home/q-posev/trexio_tools/src/converters/convert_from.py", line 769, in run
    run_pyscf(trexio_filename=trexio_filename, pyscf_checkfile=filename, back_end=back_end_str)
  File "/home/q-posev/trexio_tools/src/converters/pyscf_to_trexio.py", line 78, in pyscf_to_trexio
    assert twist_average_in == twist_average
AssertionError

Line for reproducing the error: trexio convert-from -t pyscf -i data/diamond_k_grid.chk trexio_k_grid.h5

Is it expected behavior? From what I see in the script the twist_average_in is not used anywhere else. Should I remove it together with the assertion?

q-posev commented 1 year ago

Btw, I just noticed a bug in the converter: it was writing G as basis_type attribute while it should be Gaussian, Slater or PW (see the docs for example). @kousuke-nakano you may want to update your local copy of the converter too.

kousuke-nakano commented 1 year ago

Hi @q-posev

Sorry for my late reply!

Btw, I just noticed a bug in the converter: it was writing G as basis_type attribute while it should be Gaussian, Slater or PW (see the docs for example). @kousuke-nakano you may want to update your local copy of the converter too.

Sure! Thanks. I have fixed it.

kousuke-nakano commented 1 year ago

Hi @q-posev

Integration of @kousuke-nakano 's converter into the driver script via convert-from.

In the meantime I improved error handling for the non-supported cases by raising NotImplementedError.

@kousuke-nakano I have one question: I managed to convert two of your examples (from the data folder): water.chk and diamond_single_k.chk. However, for the multiple k points I get the following error:

  File "/home/q-posev/trexio_tools/src/converters/convert_from.py", line 769, in run
    run_pyscf(trexio_filename=trexio_filename, pyscf_checkfile=filename, back_end=back_end_str)
  File "/home/q-posev/trexio_tools/src/converters/pyscf_to_trexio.py", line 78, in pyscf_to_trexio
    assert twist_average_in == twist_average
AssertionError

Line for reproducing the error: trexio convert-from -t pyscf -i data/diamond_k_grid.chk trexio_k_grid.h5

Is it expected behavior? From what I see in the script the twist_average_in is not used anywhere else. Should I remove it together with the assertion?

Hi

twist_average_in and force_wf_complex are input variables of the function pyscf_to_trexio.

def pyscf_to_trexio(
    pyscf_checkfile: str = "pyscf.chk",
    trexio_filename: str = "trexio.hdf5",
    twist_average_in: bool = False,
    force_wf_complex: bool = False,
    back_end: str = "hdf5"
):

The two variables were introduced for a safety reason, while we can eliminate them. I will do it.

q-posev commented 1 year ago

Thank you @kousuke-nakano !

There are some Git conflicts after recent reorganisation of the folders on the master branch. I might fix them later together with the bug you mentioned in #16 but I do not have time for that at the moment.

kousuke-nakano commented 1 year ago

Thank you @kousuke-nakano !

There are some Git conflicts after recent reorganisation of the folders on the master branch. I might fix them later together with the bug you mentioned in #16 but I do not have time for that at the moment.

Dear @q-posev Thank you very much! Let me know what I can do if needed!

q-posev commented 1 year ago

Hi @kousuke-nakano . I just checked and in fact file cleaning was already working correctly. The confusion comes from the fact that other converters printed a message before removing the file while pyscf-to-trexio did not. I fixed it, now it should print the name of the files before removing them.

kousuke-nakano commented 1 year ago

Hi @kousuke-nakano . I just checked and in fact file cleaning was already working correctly. The confusion comes from the fact that other converters printed a message before removing the file while pyscf-to-trexio did not. I fixed it, now it should print the name of the files before removing them.

Dear @q-posev Thanks!!