Conversation
reinit_coupled! reinitialized a coupled CellBuffer's cellvalues, dofs, and state each cell, but never refreshed its Δt. The coupled buffer therefore kept the initial NaN, or whatever value the partner had after it was last worked on its own, regardless of any set_time_increment! calls made on the partner in the meantime. Refresh each coupled buffer's Δt from its corresponding partner Simulation's base itembuffer on every reinit, before copying dofs and state. This reads the partner's current value directly rather than relying on stale scatter!'d task-local copies, and works uniformly for sequential and threaded, ordinary and autodiff buffers. Extended the "couple_buffers" regression test (test/replacements.jl) across its existing threading/autodiff/multidomain matrix to assert the coupled buffer's time increment matches the partner's value across two successive set_time_increment!+work! calls, plus a third call using an independently coupled buffer copy (via replace_material, mirroring the fracture tutorial's usage) to confirm the fix reads from the currently supplied simulation rather than any object baked in at couple_buffers time. Testing: full Pkg.test() suite passes (couple_buffers: 336/336, all other testsets green). docs/src/literate_tutorials/phasefield_fracture.jl (the package's real coupled-buffer usage) runs cleanly with no errors. Reviewed by Codex (dual-review skill, read-only independent reviewer): plan review flagged that the initial regression test only reused the same coupled buffer object already linked via couple_buffers and did not cover independently coupled copies as used in the fracture tutorial; addressed by adding the replace_material-based case above. Final diff review returned no findings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ND6mqzQ1JLuWdC8LPAhkvY
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #90 +/- ##
==========================================
+ Coverage 96.94% 96.95% +0.01%
==========================================
Files 30 30
Lines 1210 1214 +4
==========================================
+ Hits 1173 1177 +4
Misses 37 37 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Owner
|
Closed by #92 |
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.
Summary
reinit_coupled!(src/ItemBuffers/CellBuffer.jl) reinitialized a coupledCellBuffer's cellvalues, dofs, and state each cell, but never refreshed itsΔt. Depending on when the partner domain was last worked (or never), the coupled buffer's element routines could seeNaNor a stale time increment even afterset_time_increment!was called on the partner.Δtfrom the corresponding partnerSimulation's base itembuffer on every reinit, before copying dofs/state. This reads the partner's current value directly (viaget_base(get_itembuffer(sim))) rather than relying on task-local copies that are only refreshed when the partner itself is worked, and works uniformly for sequential/threaded and ordinary/autodiff buffers."couple_buffers"regression test (test/replacements.jl) across its existing threading/autodiff/multidomain matrix: two successiveset_time_increment!+work!calls with different values, plus a third call using an independently coupled buffer copy (viareplace_material, mirroring the fracture tutorial's usage pattern) to confirm the fix reads from the currently supplied simulation rather than any buffer reference baked in atcouple_bufferstime.Test plan
Pkg.test()— full suite passes,couple_bufferstestset 336/336 (up from 224/224 pre-fix, reflecting the added assertions)docs/src/literate_tutorials/phasefield_fracture.jl(the package's real coupled-buffer usage) runs cleanly with no errorscouple_buffersand didn't cover independently coupled copies as used in the fracture tutorial — addressed by adding thereplace_material-based case. Final diff review returned no findings.🤖 Generated with Claude Code
https://claude.ai/code/session_01ND6mqzQ1JLuWdC8LPAhkvY