Skip to content

[Diffusion] Explicit MechanicalObject link#6073

Open
GaetanDesrues wants to merge 3 commits into
sofa-framework:v25.12from
GaetanDesrues:v25.12
Open

[Diffusion] Explicit MechanicalObject link#6073
GaetanDesrues wants to merge 3 commits into
sofa-framework:v25.12from
GaetanDesrues:v25.12

Conversation

@GaetanDesrues

@GaetanDesrues GaetanDesrues commented Apr 8, 2026

Copy link
Copy Markdown

Adding an explicit link to the MechanicalObject instead of relying on tags

By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).

Reviewers will merge this pull-request only if

  • it builds with SUCCESS for all platforms on the CI.
  • it does not generate new warnings.
  • it does not generate new unit test failures.
  • it does not generate new scene test failures.
  • it does not break API compatibility.
  • it is more than 1 week old (or has fast-merge label).

@alxbilger alxbilger changed the title Explicit MechanicalObject link [Diffusion] Explicit MechanicalObject link Apr 8, 2026
@fredroy fredroy added pr: status to review To notify reviewers to review this pull-request pr: clean Cleaning the code labels Apr 8, 2026
fredroy
fredroy approved these changes Apr 14, 2026
View reviewed changes

@fredroy fredroy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see GaetanDesrues:v25.12 -> sofa-framework:v25.12
I think this PR should be merged into master, and might be backported to v25.12 later (or not)

@bakpaul

bakpaul commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

Hello, all ForceFields are SingleStateAccessor, which are member called mstate and already are a link to the mechanical state. Can you check if this fits your need ?

@hugtalbot hugtalbot added pr: status wip Development in the pull-request is still in progress and removed pr: status to review To notify reviewers to review this pull-request labels Apr 30, 2026
@GaetanDesrues
[LinearSolver] PCGLinearSolver: add assembled CRS specialisations
6435033 
Add PCGLinearSolver specialisations for assembled CompressedRowSparseMatrix
systems (CRS<SReal> and CRS<Mat3x3>), so the preconditioned conjugate
gradient can be used with an assembled linear system instead of only the
matrix-free GraphScattered pipeline. This makes it usable with
LinearSolverConstraintCorrection, which requires an assembled matrix.

For each CRS type, override:
  * checkLinearSystem()             -> create a MatrixLinearSystem<CRS,FV>
                                       instead of the auto-created
                                       PreconditionedMatrixFreeSystem<GraphScattered>.
  * ensureRequiredLinearSystemType()-> accept the assembled MatrixLinearSystem.
  * bwdInit()                       -> skip the PreconditionedMatrixFreeSystem linking.
  * solve()                         -> drive the preconditioner directly on the
                                       assembled matrix (precond->invert(M) once,
                                       precond->solve(M, z, r) per iteration),
                                       since a CRS preconditioner (e.g.
                                       SSORPreconditioner) has no GraphScattered
                                       setRHS/solveSystem/dispatch plumbing.

The stopping test is unchanged (preconditioned residual <r, M^-1 r> vs
tolerance * <b, b>), matching the GraphScattered implementation. Both new
templates are registered in the factory and explicitly instantiated.
@bakpaul bakpaul added pr: status to review To notify reviewers to review this pull-request and removed pr: status wip Development in the pull-request is still in progress labels Jul 3, 2026
@hugtalbot

hugtalbot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Hi @GaetanDesrues
It seems that you faced an issue to use a PCG preconditioner using a CG linear solver, then asked an AI to solve the issue. The original PR is about adding a link. However, all ForceFields are SingleStateAccessor, which are member called mstate and already are a link to the mechanical state

@GaetanDesrues

GaetanDesrues commented Jul 9, 2026

Copy link
Copy Markdown
Author

Hello @hugtalbot, yes PCG commit should not be in that PR, my mistake. I'll recreate both PRs in master.
Regarding the explicit link to the MechanicalObject, it's not targeted at the node's MechanicalObject like mstate, but rather at any MechanicalObject in the scene. It was previously addressed with tags, I'm just adding an explicit link which is cleaner than hard-coded tags.

@alxbilger

alxbilger commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

@GaetanDesrues what Hugo meant, is that TetrahedronDiffusionFEMForceField already has an explicit link to a mstate through its inheritance (base class SingleStateAccessor). Calling it with this->mstate and removing all the tag-based search should be sufficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fredroy fredroy fredroy approved these changes

Assignees

No one assigned

Labels

pr: clean Cleaning the code pr: status to review To notify reviewers to review this pull-request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@GaetanDesrues @bakpaul @hugtalbot @alxbilger @fredroy