Improve the documentation and add input checks to to_determinant_list

[q-posev]

• Closes Clarify that orbital indices should be 0-based when creating a determinant list #45

[scemama]

The orbital indices should be 0-based and not 1-based in Python: the orbital indices are of type index in trexio, which is 1-based only in Fortran. If they are 1-based in trexio-tools, it is a bug. So I think this PR is wrong. We should make the function 0-based, and shift by one what comes from resultsFile.

[q-posev]

This function has nothing to do with TREXIO format per se. It is a helper function to convert a list of occupied orbitals into the determinant bit string. We do not store lists of occupied orbitals per determinant in TREXIO.

[scemama]

I agree, but I think we should have this function consistent with the helper functions of trexio trexio_to_orbital_list_up_dn and keep the logic that everything is 0-based in Python.

[q-posev]

This would be a rather trivial change, I can do that.
So just to confirm: indices should be 0-based when provided to the function and in the function we shift them by +1 so that the current code remains unchanged?

[scemama]

Yep!

…

On Sun, 2024-11-10 at 09:41 -0800, Evgeny Posenitskiy wrote: This would be a rather trivial change, I can do that. So just to confirm: indices should be 0-based when provided to the function and in the function we shift them by +1 so that the current code remains unchanged? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because your review was requested.Message ID: ***@***.***>

[q-posev]

Hello again @NastaMauger,
In this PR, we decided to change to_determinant_list to work with 0-based indices (as you originally tried to do with PySCF). We will shift the indices internally as needed.
It's a bit tricky to sync with your development, but once this PR is merged - your code for transforming the orbitals from occsa and occsb would need to be adapted to the way it was originally in your code here (i.e. trexio_det.to_determinant_list(occsa, int64_num)). Sorry for the inconvenience, but it's all for the best (@scemama is right about the need to ensure consistency of indices everywhere).

[q-posev]

I am done with the changes, so ready to merge whenever is more convenient for you @NastaMauger and @scemama

[NastaMauger]

@q-posev I have modified my PR to take into account your comment regarding the 0-based indices and the electron group writing information. Does this meet your requirements?

[q-posev]

@NastaMauger that was fast! Yes, it's perfect, thanks a lot! I left a small comment in your PR.

We will merge this MR ASAP together with #48 and will make the release of trexio-tools package so that pyscf-forge can integrate the right version into their dependency stack.