-
Notifications
You must be signed in to change notification settings - Fork 63
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
PBC fix bundle #563
base: master
Are you sure you want to change the base?
PBC fix bundle #563
Conversation
… nuclei programmatically
…lution can be controlled by the user, if needed; the default is unchanged (truncate the expansion if periodic, not otherwise)
…istances using distsq_periodic
…sider periodic displacements since operator kernel includes lattice summation
src/madness/mra/operator.h
Outdated
@@ -1061,7 +1050,7 @@ namespace madness { | |||
ops[mu].setfac(coeff(mu)/c); | |||
|
|||
for (std::size_t d=0; d<NDIM; ++d) { | |||
ops[mu].setop(d,GaussianConvolution1DCache<Q>::get(k, expnt(mu)*width[d]*width[d], 0, isperiodicsum)); | |||
ops[mu].setop(d,GaussianConvolution1DCache<Q>::get(k, expnt(mu)*width[d]*width[d], 0, lattice_sum[d])); |
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.
@robertjharrison while trying to deal with FunctionImpl::do_apply PBC issues I ended up tackling the mixed BC handling in SeparatedConvolution ... is there more to it than just toggling lattice sum flag for each setop
? A pair of eyes on this commit would be good. @JonathonMisiewicz u2
Huh.
Prolly also how the operator displacements are generated in func defaults
(?). Also if the aspect ratio is far from cubic (e.g., finite slab width
is many multiples of the periodic width) then the sorting of displacements
and the screening algorithm in do apply could be optimized (but I think
this is just performance).
I can read thru this later in the week.
…On Tue, Dec 3, 2024, 20:10 Eduard Valeyev ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/madness/mra/operator.h
<#563 (comment)>
:
> @@ -1061,7 +1050,7 @@ namespace madness {
ops[mu].setfac(coeff(mu)/c);
for (std::size_t d=0; d<NDIM; ++d) {
- ops[mu].setop(d,GaussianConvolution1DCache<Q>::get(k, expnt(mu)*width[d]*width[d], 0, isperiodicsum));
+ ops[mu].setop(d,GaussianConvolution1DCache<Q>::get(k, expnt(mu)*width[d]*width[d], 0, lattice_sum[d]));
@robertjharrison <https://github.com/robertjharrison> while trying to
deal with FunctionImpl::do_apply PBC issues I ended up tackling the mixed
BC handling in SeparatedConvolution ... is there more to it than just
toggling lattice sum flag for each setop? A pair of eyes on this commit
would be good. @JonathonMisiewicz <https://github.com/JonathonMisiewicz>
u2
—
Reply to this email directly, view it on GitHub
<#563 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABZSAPN7RWJ5AF2FQT2FDUL2DZI7ZAVCNFSM6AAAAABS67KN6WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDINZXGA2TKMBVG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
7373da9
to
5fa8afe
Compare
I went through that code, I think it's fine. I presume we only want to generate displacements for 2 cases only: all non-periodic and all-periodic, with mixed cases handled by the periodic displacements (optimizing for mixed case would just reduce the number of replacements). Or, do we want to generate the displacements for the specific boundary conditions FunctionDefaults was initialized for (i.e. assume the boundary conditions are immutable?). I think the spherical shell-based screening logic should be OK in
I also touched that code, and should be OK. |
…undary conditions
… R!=T, constexpr avoid those paths
… provides probing_displacement method to enable screening out the entire surface contribution
…acement as "used" if it survived estimated AND actial norms of its contribution
…o the entire surface of the boundary range, rather than to near the facet centers
…e factory and the functor
… FunctionFunctorInterface children
…eriodic potential for the tight (nuclear-sized) densities
…or testing/logging purposes
… std::copy to invoke proper conversion Q->T
…ntRangeV2 which are no longer used
519ba6a
to
9c310a0
Compare
…is out of range filter function generalized to take Key pattern instead of Key; displacement is passed as an optional
9c310a0
to
63a3c85
Compare
…ialFunctor with Poisson kernel range restricted, and optional support for periodic boundary conditions
displacements along nonfixed axes of finite radius did not take into account for the finite surface thickness.
nonperiodic standard displacements, although screened in units of spherical shells, are defined in cubic sets, this erroneously skipped displacements with one of the Cartesian components > bmax but distsq less than the max reached by standard displacements implicitly lattice-summed range-restricted operator (RP) still not correct, but at least its explicit version (RP2) is
…ndle "periodic" dimensions (those long which the operator has been lattice summed) for periodic dimensions BoxSurfaceDisplacementRange only produces displacements that are unique modulo this period
ee3ad43
to
d78aee1
Compare
rscale