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

1-e σpVσp #273

Merged
merged 22 commits into from
Dec 6, 2023
Merged

1-e σpVσp #273

merged 22 commits into from
Dec 6, 2023

Conversation

evaleev
Copy link
Owner

@evaleev evaleev commented Oct 30, 2023

No description provided.

@evaleev evaleev marked this pull request as draft October 30, 2023 18:12
@JonathonMisiewicz
Copy link
Collaborator

JonathonMisiewicz commented Nov 17, 2023

Active Todo List for pVp Integrals:

  • Figure out how CR interacts with engine.impl.h::compute_1

@loriab
Copy link
Collaborator

loriab commented Nov 29, 2023

Alas, the sigmas appear in filenames in the export. Windows couldn't handle * in basis set filenames, so I'm not surprised it's balking on the non-ascii.

files

_sphemultipole_S_S_prereq.cc
_σpVσp_D_D.cc
_σpVσp_D_D_prereq.cc
_σpVσp_D_P.cc
_σpVσp_D_P_prereq.cc
_σpVσp_D_S.cc
_σpVσp_D_S_prereq.cc
_σpVσp_P_D.cc
_σpVσp_P_D_prereq.cc
_σpVσp_P_P.cc
_σpVσp_P_P_prereq.cc
_σpVσp_P_S.cc
_σpVσp_P_S_prereq.cc
_σpVσp_S_D.cc
_σpVσp_S_D_prereq.cc
_σpVσp_S_P.cc
_σpVσp_S_P_prereq.cc
_σpVσp_S_S.cc
_σpVσp_S_S_prereq.cc
configuration.cc.cmake.in

lines

_σpVσp_D_D.cc:#include <_σpVσp_D_D_prereq.h>
_σpVσp_D_D.cc:void _σpVσp_D_D(const Libint_t* inteval) {
_σpVσp_D_D.cc:_σpVσp_D_D_prereq(inteval+c, inteval->stack);
_σpVσp_D_D_prereq.cc:void _σpVσp_D_D_prereq(const Libint_t* inteval, LIBINT2_REALTYPE* parent_stack) {
libint2_static_init.cc:libint2_build_σpVσp[2][2] = _σpVσp_D_D;

@evaleev
Copy link
Owner Author

evaleev commented Nov 29, 2023

@loriab is this something that the compiler does when interpreting the source? or is this something that cmake does when making a unity source? trying to disable unity build to see if this has any effect ...

@evaleev
Copy link
Owner Author

evaleev commented Nov 29, 2023

I don't think unicode characters in file names should be a problem ... * is a different story: https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions

@evaleev evaleev force-pushed the feature/σpVσp branch 5 times, most recently from 9c6056a to 37fceea Compare November 29, 2023 20:34
@loriab
Copy link
Collaborator

loriab commented Nov 29, 2023

Longshot (why would the basis fn be called?) but easy to try if removing https://github.com/evaleev/libint/blob/master/include/libint2/basis.h.in#L348-L350 helps. Also note that if you want a shorter time to failure for GHA, can do https://github.com/evaleev/libint/blob/master/.github/workflows/cmake.yml#L161-L165 . Sorry for all the trouble Wiindows is causing.

@JonathonMisiewicz
Copy link
Collaborator

JonathonMisiewicz commented Nov 29, 2023

And with the latest commit, Mathematica and L2 and my-script-using-L2-V-ints all match in Cartesians.

AFAIK, what remains on this PR is:

  • Check signs against MPQC
  • Write test case
  • Fix Windows export
  • Final cleanup

@evaleev
Copy link
Owner Author

evaleev commented Nov 30, 2023

Nice, was thereva real issue with D integrals then?

@evaleev
Copy link
Owner Author

evaleev commented Nov 30, 2023

indeed: this suggests condo builds ninja using configure.py but looks like one must use CMake and MSVC to build ninja to support UTF!

So, indeed, UTF-8 support is not possible on Windows without requiring properly built ninja. P.S. What is wrong with anyone choosing to use Windows?

@evaleev
Copy link
Owner Author

evaleev commented Nov 30, 2023

@loriab the library now builds but python module does not ... https://github.com/evaleev/libint/actions/runs/7049909124/job/19189926846#step:7:13369

@JonathonMisiewicz
Copy link
Collaborator

exec-charset is Linux. execution-charset is Windows. 🙃

@loriab
Copy link
Collaborator

loriab commented Nov 30, 2023

It might be worth disabling Win / Python in extract+build+install to ensure that the units tests and the HF are still good for windows with the symlink. It'll still fail on the Build& Test Python bindings where it builds pylibint again.

@loriab
Copy link
Collaborator

loriab commented Nov 30, 2023

@@ -284,7 +284,7 @@ jobs:
-DLIBINT2_PYTHON=ON \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lori suggested turning this OFF if we're still having problems even after this latest version.

@evaleev
Copy link
Owner Author

evaleev commented Nov 30, 2023

looks like the linking always fails ... https://github.com/evaleev/libint/actions/runs/7052599388/job/19198357371#step:8:78

guys, feel free to try but I'm ready to give up. This is extremely annoying.

@loriab
Copy link
Collaborator

loriab commented Nov 30, 2023

looks like the linking always fails ... https://github.com/evaleev/libint/actions/runs/7052599388/job/19198357371#step:8:78

guys, feel free to try but I'm ready to give up. This is extremely annoying.

Definitely annoying. Best suggestion I have is having eye-friendly and file-friendly versions of the operator strings -- former for the code one looks at and latter for anything generated, for any OS, though the offender is Windows.

@JonathonMisiewicz
Copy link
Collaborator

The only other idea I have at this point is to try conda from pip rather than conda - the pip version goes through cmake and should behave correctly without symlink trickery. I don't know if that's acceptable for Lori's purposes.

@evaleev
Copy link
Owner Author

evaleev commented Nov 30, 2023

The only other idea I have at this point is to try conda from pip rather than conda - the pip version goes through cmake and should behave correctly without symlink trickery. I don't know if that's acceptable for Lori's purposes.

you mean pip can provide ninja built with cmake?

@loriab
Copy link
Collaborator

loriab commented Nov 30, 2023

The only other idea I have at this point is to try conda from pip rather than conda - the pip version goes through cmake and should behave correctly without symlink trickery. I don't know if that's acceptable for Lori's purposes.

you mean pip can provide ninja built with cmake?

That's what I glean from https://github.com/ninja-build/ninja/blob/master/.github/workflows/windows.yml

The reservation Jonathon mentioned is that for c-f I need to be able to build the libint recipe from a controlled environment. They're not going to care for pinning consistency about a tool that gets consumed (like the ninja generator), but it may be tricky to get ahold of the software not from the c-f channel. But in terms of figuring out will-cmake-built-win-ninja fix this problem, worth a try.

@evaleev
Copy link
Owner Author

evaleev commented Dec 1, 2023

bye Operator::σpVσp 😭 hope to see you back soon

@JonathonMisiewicz
Copy link
Collaborator

JonathonMisiewicz commented Dec 1, 2023

This is needed to agree with the MPQC formulas.

Presumably need to update unit tests?

@JonathonMisiewicz
Copy link
Collaborator

I'm pretty sure you meant to start a new post rather than edit mine. But yes, I'm working on generating the test data now.

@JonathonMisiewicz JonathonMisiewicz marked this pull request as ready for review December 1, 2023 15:35
@@ -203,7 +217,7 @@ target_compile_features(libint2_obj PUBLIC "cxx_std_11")
set_target_properties(
libint2_obj
PROPERTIES
UNITY_BUILD TRUE
UNITY_BUILD FALSE
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
UNITY_BUILD FALSE
UNITY_BUILD TRUE

avoids too many files for high AM

@loriab loriab mentioned this pull request Dec 3, 2023
Copy link
Owner Author

@evaleev evaleev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r2g

@evaleev evaleev merged commit 2586798 into master Dec 6, 2023
8 checks passed
@evaleev evaleev deleted the feature/σpVσp branch December 6, 2023 17:23
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.

3 participants