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

[WIP] clearer trexio eri interface #105

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

Conversation

kgasperich
Copy link

This is just one example of a way to address the issues brought up in Issue #103

The changes here are just the first implementation I thought of that adds some clarity/safety, so I'm not strongly attached to the way I've done it here, and someone more familiar with pyscf might know of a cleaner way to handle it, especially regarding the ao2mo part. I'm not familiar with all of the possible options for input/output symmetries, so it might make sense to pass some more args through to that function to allow for various possibilities.

I also haven't changed/added any tests here, but I could do that if there is any interest in merging the PR.

@NastaMauger
Copy link
Contributor

NastaMauger commented Mar 1, 2025

Hello @kgasperich

I think to work effectively, we might want to add a chemist/physicist notation feature, either directly in TREXIO or within the PySCF-Tools. This would help standardize how we handle the ERIs and ensure clarity when dealing with different notation/softwares.

In my PR. after the write_eri function, I added the following:

idx=idx.reshape((num_integrals,4))
for i in range(num_integrals):
    idx[i,1],idx[i,2]=idx[i,2],idx[i,1] 

idx=idx.flatten()

with trexio.File(filename, 'w', back_end=_mode(backend)) as tf: 
    trexio.write_mo_2e_int_eri(tf, 0, num_integrals, idx, eri.ravel())

I believe this change should be incorporated. Let me know your thoughts.

Best

@kgasperich
Copy link
Author

Hi @NastaMauger

That change should work to swap between phys/chem notation.
I think with numpy slicing you can accomplish the same thing with idx[:,[1,2]] = idx[:,[2,1]] to avoid the loop, but it's really just a style preference, and it's fine either way.

Also, I think idx.flatten() will return a copy (whereas idx.ravel() would return a view). It's probably not worth worrying about, but maybe there are cases where avoiding the copy would be better.

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.

2 participants