-
Notifications
You must be signed in to change notification settings - Fork 147
Add kspace jastrow batched test #5353
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
Conversation
Test this please |
qmc_short_kspace_4_4 | ||
qmc_short_kspace_4_4.in.xml | ||
4 | ||
4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test seems being added legacy only. If we only intend to have this test either legacy or batch, I think batch is preferred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The batched driver test is added at line 103. The ones above are not new, except I changed them to always run. Since the tests are fast, I think we should keep both tests to reduce the chance for problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Do you plan to add this 4x4 case to batched as well in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can if you would like it added. Since the Jastrows involve no parallelism, I generally don't think they add much. Your call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do it in this PR and we won't rediscover missing coverage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added. Also added samples check.
Test this please |
Proposed changes
Add batched version of existing short test.
Also, remove offload checks in CMakeLists.txt
In part motivated by #5348 . The new test passes on CPUs.
What type(s) of changes does this code introduce?
Does this introduce a breaking change?
What systems has this change been tested on?
Ubuntu24, AOCC+AOCL
Checklist