Fix BUG-013: generalize create_chunks off concrete Ferrite.Grid - #104
Merged
Merged
Conversation
Allow threading=true for any Ferrite.AbstractGrid, not just concrete Grid, so external grid implementations (e.g. FerriteIGA.jl's BezierGrid) can use threaded assembly. src/Multithreading/TaskChunks.jl: relax all four create_chunks method signatures from ::Ferrite.Grid to ::Ferrite.AbstractGrid. None of them use Grid-specific fields; automatic coloring already goes through Ferrite.create_coloring, which accepts AbstractGrid, and the user-supplied-chunks overload never used the grid argument beyond dispatch. test/threading_utils.jl: add a unit test with a minimal non-Grid AbstractGrid wrapper (only .cells/.nodes fields, matching Ferrite's generic AbstractGrid accessors) exercising both the automatic-coloring and user-supplied-chunks create_chunks paths. docs/src/literate_tutorials/iga.jl: add a threaded-assembly section using the real FerriteIGA.jl BezierGrid, comparing K/r against the sequential result. Uses num_tasks=2 explicitly (independent of Threads.nthreads()) so the chunk-splitting logic is exercised even in a single-threaded session; the comparison lines use #hide (not #src) so they survive into the generated script/markdown that Documenter actually executes, not just local manual runs. .github/workflows/CI.yml: add JULIA_NUM_THREADS: 2 to the docs job, matching the test job, so the threaded comparison in the IGA tutorial is exercised with real concurrency in CI (previously the docs job ran with Julia's default of 1 thread). Reviewed via /dual-review with Codex (read-only independent reviewer): - Plan review caught that the docs job runs single-threaded in CI, which would have left the new regression check untested there -> added JULIA_NUM_THREADS: 2 to the docs job. - Final-diff review caught that the two @test lines were marked #src, which Literate strips from every generated output (script, markdown, notebook) -> switched to #hide (this repo's established pattern, e.g. docs/src/literate_howto/automatic_differentiation.jl) so the assertions actually run as part of the Documentation CI job. - Second final-diff review after that fix: no further findings. Validation: Pkg.test() passes (all suites, including new BUG-013 unit test); every tutorial and how-to under docs/src/literate_* runs cleanly; full docs/make.jl build completes with no @example block errors. Also validated BUG-024 (docs/make.jl world-age failures) as an environmental issue, not a source bug: reproduced it live via a competing long-lived Julia process still attached to the docs project, then confirmed a clean docs/make.jl build once no competing process remained. Updated identified_bugs.md accordingly; BUG-013 marked done and its open-item section removed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KySgpVgJ5fWR9AvPK1JQoU
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #104 +/- ##
=======================================
Coverage 97.31% 97.31%
=======================================
Files 32 32
Lines 1376 1376
=======================================
Hits 1339 1339
Misses 37 37 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
KnutAM
reviewed
Sep 30, 2026
|
|
||
| # Start by loading the necessary packages | ||
| using Ferrite, FerriteIGA, LinearAlgebra, FerriteAssembly | ||
| using Test #src |
| write_solution(vtk, dh, a) | ||
| end | ||
|
|
||
| using Test #src |
Contributor
Author
There was a problem hiding this comment.
Good catch — switched to #hide in f64c3ab, same fix as the threaded-assembly asserts earlier in the diff (found the same #src-strips-everything issue via Codex review).
…istently Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KySgpVgJ5fWR9AvPK1JQoU - Removed the redundant `using Test #src` at the top of iga.jl: it was stripped from every generated output anyway (Literate removes #src lines unconditionally), and the tutorial's own `using Test #hide` right before the threaded-assembly assertions already covers all @test lines further down in the file. - Switched the pre-existing bottom-of-file `@test norm(norm.(qe.data)) ≈ 679.3207411544098` from #src to #hide, for the same reason as the threaded-assembly assertions fixed earlier in this PR: #src strips the line from every generated script/markdown/notebook, so it never actually runs as part of the Documentation CI job. Validation: re-ran the iga.jl tutorial standalone and the full docs/make.jl build (both clean, no errors); Pkg.test() passes (all suites, including the BUG-013 unit test).
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
2 participants
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
create_chunksinsrc/Multithreading/TaskChunks.jlnow dispatches onFerrite.AbstractGridinstead of the concreteFerrite.Grid, sothreading=trueworks for external grid implementations such as FerriteIGA.jl'sBezierGrid. None of the methods usedGrid-specific fields;Ferrite.create_coloringalready acceptsAbstractGrid, and the user-supplied-chunks overload never used the grid argument beyond dispatch.test/threading_utils.jlwith a minimal non-GridAbstractGridwrapper, exercising both the automatic-coloring and user-supplied-chunks paths.docs/src/literate_tutorials/iga.jlusing the realFerriteIGA.jlBezierGrid, comparing threaded vs. sequentialK/r(num_tasks=2explicit, independent ofThreads.nthreads(), so the chunk-splitting logic is exercised even in a single-threaded session).JULIA_NUM_THREADS: 2to the docs CI job in.github/workflows/CI.ymlso this comparison runs with real concurrency in CI (previously single-threaded).Review notes
Reviewed via a read-only independent Codex pass at plan and final-diff stages:
JULIA_NUM_THREADS: 2.@testlines were marked#src, which Literate strips from every generated output (script/markdown/notebook), so the assertions would never actually run as part of the Documentation CI job → switched to#hide(this repo's established pattern, e.g.docs/src/literate_howto/automatic_differentiation.jl), which keeps the code in generated artifacts while hiding it from the rendered docs page.Test plan
Pkg.test()passes (all suites, including the new BUG-013 test, 3/3)docs/src/literate_tutorials/docs/src/literate_howtoruns cleanly in isolationdocs/make.jlbuild completes with zero@exampleblock errors🤖 Generated with Claude Code
https://claude.ai/code/session_01KySgpVgJ5fWR9AvPK1JQoU