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

Laplace3d bugfix backport #2242

Conversation

johnomotani
Copy link
Contributor

Backport of #2237, and extends the bugfix to LaplaceHypre3d.

Also includes a fix for operator+=() in the Hypre interface, which was previously over-writing values instead of adding to them.

johnomotani and others added 18 commits February 22, 2021 20:24
These are occasionally needed, and it only adds initialisation cost to
calculate them.
The method actually creates the intersection.

Fixes #2236.
getUnion() was misleadingly named, and actually gave the intersection.
Region::operator+=(Region) updates a Region to be the union of two
Regions, which is almost what was wanted, although we have to call
mask_region.unique() afterwards too to ensure there are no repeated
entries and the entries are sorted.
Stencil was storing a lambda which captured some state by reference,
which went out of scope before the lambda was called. Fix is to
capture by value. `RangeIterator` is not particularly expensive to
copy, so this should not affect performance.

Possible issue with linked `RangeIterator`s?
Fix copied from fix to PETSc interface in
dd8bed9:

Stencil was storing a lambda which captured some state by reference,
which went out of scope before the lambda was called. Fix is to
capture by value. `RangeIterator` is not particularly expensive to
copy, so this should not affect performance.

Possible issue with linked `RangeIterator`s?
Like recent fix to test-laplace-petsc3d, paralleltransform option was in
wrong section.
Previously operator+=() called setValues(), which overwrites any
existing value in the matrix element. It should add to existing values,
so now calls new addValues() method, which in turn calls new
HypreMatrix::addVal() method.
examples/laplacexy/simple doesn't compile under Cuda. In the very short term just to test this PR it is commented out in Top level CMakeLists.txt.  This app needs changes to its CMakeLists.txt or should not be compiled under Cuda - both are very easy fixes. It will be remedied in next update to next-hypre-outerloop-cuda
@jonesholger
Copy link
Collaborator

elmpb is still unstable while running against laplacexy-hypre. While I do have a few stability fixes in the works, we absolutely need to work on initialization and assembly checks. But, this PR looks good with the couple of inline edits I added earlier.

@jonesholger jonesholger merged commit af73d70 into next-hypre-outerloop-cuda Feb 28, 2021
@johnomotani johnomotani deleted the next-hypre-outerloop-cuda-laplace3d-bugfix-backport branch February 28, 2021 17:36
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.

None yet

3 participants