Conversation
…ies (BUG COM5) - Updated multiple `fields` and `particles` assignments to ensure `.copy()` is used when converting to `np.asarray`, preventing potential issues with memory views. - Adjusted handling of `bondDist` to avoid creating problematic views during inplace modifications. - Added tests to verify that arrays returned by `updateFieldsC` have `owndata=True` and no base reference.
samosProjectionIteratrive: fix Lxi/Lyi never updated inside loop, causing convergence check to always compare against initial cell (perf-only, no correctness change). distConservProjectionIteratrive: add distn==0 guard before dist/distn to prevent NaN for stopped particles.
removedBonds now builds old-to-new particle index mapping and remaps both bondStart (reindexed by new particle indices) and bondPart (target indices remapped). removePart skips bond keys since they are already correctly rebuilt by removedBonds. Previously bondPart stored original indices that became stale after np.extract compacted particle arrays, causing silent data corruption or out-of-bounds access when particles exited the domain.
particles["stoppedParticles"] = particles created a self-referencing dict that overwrote the correct stopped-particle arrays (x, y, m, ID) with the full flowing-particle data. Downstream in updateFieldsC this caused stopped mass rasters to use flowing instead of stopped masses. Also prevented garbage collection and broke serialization. Removed the offending line. stoppedParticles is already correctly populated earlier in the function, and massStopped captures the accumulated total.
…ionsCython` Updated array initializations (`pfv`, `ppr`, `pft`, `pta`, `pke`) to use dynamic dimensions based on `header["nrows"]` and `header["ncols"]` instead of static `(1, 1)` arrays. This ensures compatibility with test inputs of varying sizes.
Contributor
Analysis for project
|
| Tool | Category | Rule | Count | |
|---|---|---|---|---|
| black | Style | Incorrect formatting, autoformat by running qlty fmt. |
1 | ❌ |
@qltysh one-click actions:
- Auto-fix formatting (
qlty fmt && git push)
Contributor
|
Coverage Impact This PR will not change total coverage. Modified Components (1)
Modified Files with Diff Coverage (1)
🛟 Help
|
fso42
changed the title
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
1 participant
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.
Reported by CT (com5 hang on althof testcase). During analysis other problems cropped up, which are fixed now
Summary
Fixes several bugs in com1DFA discovered during simulations: circular reference in stopped particles, stale bond indices after particle removal, out-of-bounds heap corruption from mismatched test array sizes, plus hardening against NaN/div-by-zero in reprojection.
Changes
1. Remove circular reference in
stoppedParticles(1793088)particles["stoppedParticles"] = particlescreated a self-referencing dict that overwrote the correct stopped-particle arrays (x, y, m, ID) with full flowing-particle data. Downstream inupdateFieldsCthis caused stopped mass rasters to use flowing instead of stopped masses. It also prevented garbage collection and broke serialization. The line was removed;stoppedParticlesis already correctly populated earlier.2. Remap bond indices after particle removal (ff7594d)
removedBondsnow builds an old-to-new particle index mapping and remaps bothbondStart(reindexed by new particle indices) andbondPart(target indices remapped).removePartskips bond keys since they are already rebuilt. PreviouslybondPartstored original indices that became stale afternp.extractcompacted arrays, causing silent data corruption or out-of-bounds access.3. Fix convergence check and division-by-zero in reprojection (2903c04)
samosProjectionIteratrive: fixedLxi/Lyinever updated inside the loop, causing the convergence check to always compare against the initial cell (performance-only, no correctness change).distConservProjectionIteratrive: addeddistn == 0guard beforedist / distnto prevent NaN for stopped particles.4. Ensure arrays returned by
updateFieldsCare owned copies (ab23111)Multiple
fieldsandparticlesassignments now use.copy()when converting tonp.asarray, preventing issues with memory views that could lead to dangling references or double-free (BUG COM5). AdjustedbondDisthandling to avoid problematic views during inplace modification. Added tests verifyingowndata=Trueandbase is None.