fix: enforce now handles rows with no nonzeros (fixes #1195)#1212
Merged
Conversation
`enforce` zeros the rows of `A` belonging to the Dirichlet dofs `D` by
building a flat array of positions into `A.data`. The previous
implementation did this incrementally:
idx = np.ones(count.sum(), dtype=np.int32)
idx[np.cumsum(count)[:-1]] -= count[:-1]
idx = np.repeat(start, count) + np.cumsum(idx) - 1
That works only when every row in `D` has at least one stored nonzero.
If any `count[i] == 0` -- which is what happens whenever an element has
only Dirichlet dofs, e.g. a `MeshTri().refined(1)` p-Laplacian Hessian
whose surface dofs have no in-bulk neighbours -- then
`np.cumsum(count)[:-1]` contains duplicate indices, the corresponding
entries in `idx` are decremented twice, and the final `idx` runs past
the end of `A.data` raising `IndexError`.
Rewrite the index construction to handle empty rows correctly:
offsets = (np.arange(count.sum())
- np.repeat(np.cumsum(count) - count, count))
idx = np.repeat(start, count) + offsets
`np.repeat(start, count)` and the offset arithmetic both contribute
nothing for `count[i] == 0`, so the result is well-defined regardless
of how many zero-rows are in `D`. Also short-circuit when
`count.sum() == 0` (e.g. `D` is empty), avoiding scipy warnings about
zero-length fancy indexing.
Adds `test_enforce_handles_empty_rows` exercising the failing scenario
(a `BilinearForm` whose coefficient is identically zero gives every
boundary row no nonzeros), and verifies the enforced rows have
`diag = 1` and no off-diagonal nonzeros while non-enforced rows are
untouched, plus the right-hand side correctness.
Owner
|
Thank you for the contribution! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix:
enforcenow handles rows with no nonzerosFixes #1195.
Problem
skfem.utils.enforcezeros the rows ofAbelonging to the Dirichletdofs
Dby constructing a flat array of positions intoA.data. Theprevious implementation:
only works when every row in
Dhas at least one stored nonzero. If anycount[i] == 0— which happens whenever an enforced row has no in-bulkneighbours, e.g. a
MeshTri().refined(1)p-Laplacian Hessian whosesurface dofs have all-zero gradient contributions — then
np.cumsum(count)[:-1]contains duplicate indices, the correspondingentries in
idxare decremented twice, and the finalidxruns pastthe end of
A.dataand raisesIndexError.Repro from #1195 (a p-Laplacian Hessian on a 1-refined mesh, where the
surface dofs end up with empty rows):
Different from the closed #1043 (where the entire matrix is zero and
the result wouldn't be invertible) — here only a subset of the enforced
rows are empty and the resulting enforced system is perfectly usable.
Fix
Rewrite the index construction to handle empty rows correctly:
Both
np.repeat(start, count)and the offset arithmetic contributenothing when
count[i] == 0, so the result is well-defined regardlessof how many zero-rows are in
D. Also short-circuit whencount.sum() == 0(e.g.Dis empty) to avoid scipy warnings aboutzero-length fancy indexing.
Verification
Adds
tests/test_utils.py::test_enforce_handles_empty_rows. The testsets up the failing scenario explicitly (a
BilinearFormwhosecoefficient is identically zero, which makes every boundary row empty)
and asserts:
assert (counts == 0).any()— the test really triggers the brokencode-path,
diag == 1.0and no off-diagonal nonzeros,bout[D] == x[D]and other entries unchanged.Local CI (matches
.github/workflows/main.yml):Scope
Only
skfem/utils.py(production fix) andtests/test_utils.py(regression test). No public API change —
enforce's signature andreturn contract are unchanged; the fix is purely the internal flat-index
construction.