Skip to content

Fix stale time increment in coupled buffers (BUG-003) - #90

Closed
knutambot wants to merge 1 commit into
KnutAM:mainfrom
knutambot:cb/BUG-003
Closed

knutambot wants to merge 1 commit into
KnutAM:mainfrom
knutambot:cb/BUG-003

Conversation

@knutambot

Copy link
Copy Markdown
Contributor

Summary

  • reinit_coupled! (src/ItemBuffers/CellBuffer.jl) reinitialized a coupled CellBuffer'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 see NaN or a stale time increment even after set_time_increment! was called on the partner.
  • Fix: refresh each coupled buffer's Δt from the corresponding partner Simulation's base itembuffer on every reinit, before copying dofs/state. This reads the partner's current value directly (via get_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.
  • Extended the "couple_buffers" regression test (test/replacements.jl) across its existing threading/autodiff/multidomain matrix: two successive set_time_increment! + work! calls with different values, plus a third call using an independently coupled buffer copy (via replace_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 at couple_buffers time.

Test plan

  • Pkg.test() — full suite passes, couple_buffers testset 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 errors
  • Reviewed by Codex as an independent, read-only reviewer (dual-review skill): plan review flagged that the initial test only reused the buffer object already linked via couple_buffers and didn't cover independently coupled copies as used in the fracture tutorial — addressed by adding the replace_material-based case. Final diff review returned no findings.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ND6mqzQ1JLuWdC8LPAhkvY

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

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.95%. Comparing base (485e596) to head (49ed886).
⚠️ Report is 1 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KnutAM

KnutAM commented Sep 17, 2026

Copy link
Copy Markdown
Owner

Closed by #92

@KnutAM KnutAM closed this Sep 17, 2026
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.

2 participants