Skip to content

Refactor LA into native and PETSc part and add assigners - #3659

Merged
jhale merged 51 commits into
mainfrom
jhale/refactor-la-petsc
Mar 6, 2025
Merged

Refactor LA into native and PETSc part and add assigners#3659
jhale merged 51 commits into
mainfrom
jhale/refactor-la-petsc

Conversation

@jhale

@jhale jhale commented Mar 5, 2025

Copy link
Copy Markdown
Member

Supersedes #3652 which made it apparent that dolfinx.la should have a dedicated dolfinx.la.petsc module.

This PR introduces API changes:

dolfinx.la.create_petsc_vector -> dolfinx.la.petsc.create_vector
dolfinx.la.create_petsc_vector_wrap -> dolfinx.la.petsc.create_vector_wrap

Note that dolfinx.fem.create_vector is now a very light wrapper over dolfinx.la.create_vector, perhaps bringing into question the need for the former.

jhale and others added 30 commits February 27, 2025 15:43
Co-authored-by: Jørgen Schartum Dokken <dokken92@gmail.com>
Co-authored-by: Jørgen Schartum Dokken <dokken92@gmail.com>
Co-authored-by: Jørgen Schartum Dokken <dokken92@gmail.com>
Co-authored-by: Jørgen Schartum Dokken <dokken92@gmail.com>
A Collection has no ordering - Sequence is correct.
Use np.inexact instead of np.floating
@jorgensd

jorgensd commented Mar 5, 2025

Copy link
Copy Markdown
Member

Couldn’t we place all PETSC related structures in dolfinx.petsc instead of having spread namespaces as nls,fem,la that has all to be explicitly imported in DOLFINx?

@jhale

jhale commented Mar 6, 2025

Copy link
Copy Markdown
Member Author

Couldn’t we place all PETSC related structures in dolfinx.petsc instead of having spread namespaces as nls,fem,la that has all to be explicitly imported in DOLFINx?

Well, yes, possibly, but not in this PR. But it's a huge API change. Let's discuss it on Slack.

Comment thread python/dolfinx/fem/petsc.py

@michalhabera michalhabera 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.

The separation into fem.petsc and la.petsc reads very nice! And also follows the C++ structure.

Why are all these helpers (la.petsc.create_vector_wrap, la.petsc.create_vector, la.petsc.assign, fem.petsc.assign, ...) written in Python only? Is the argument that C++ users would not need/want such helpers?

Comment thread python/test/unit/fem/test_petsc_nonlinear_assembler.py Outdated
Comment thread python/test/unit/fem/test_petsc_nonlinear_assembler.py Outdated
Comment thread python/dolfinx/fem/petsc.py
Comment thread python/dolfinx/la/__init__.py
Comment thread python/dolfinx/fem/petsc.py Outdated
Comment thread python/dolfinx/la/petsc.py
@jhale

jhale commented Mar 6, 2025

Copy link
Copy Markdown
Member Author

Why are all these helpers (la.petsc.create_vector_wrap, la.petsc.create_vector, la.petsc.assign, fem.petsc.assign, ...) written in Python only? Is the argument that C++ users would not need/want such helpers?

I discussed this with @garth-wells - providing similar functionality for C++ users would be beneficial as interfacing SNES in particular is apparently "very hard". However, it's not clear we would want to write helpers in C++ using PETSc and then expose to Python, or just provide two good designs that suits C++ and Python via PETSc C and petsc4py, respectively.

Co-authored-by: Michal Habera <michal.habera@gmail.com>
@garth-wells

Copy link
Copy Markdown
Member

Why are all these helpers (la.petsc.create_vector_wrap, la.petsc.create_vector, la.petsc.assign, fem.petsc.assign, ...) written in Python only? Is the argument that C++ users would not need/want such helpers?

I discussed this with @garth-wells - providing similar functionality for C++ users would be beneficial as interfacing SNES in particular is apparently "very hard". However, it's not clear we would want to write helpers in C++ using PETSc and then expose to Python, or just provide two good designs that suits C++ and Python via PETSc C and petsc4py, respectively.

There is a helper at

std::vector<std::vector<PetscScalar>> get_local_vectors(
.

More C++ 'helpers' could be helpful, but we should do it with a purpose, e.g. when adding SNES helpers, to get it right rather than adding it for hypothetical reasons.

@jhale
jhale enabled auto-merge March 6, 2025 19:00
@jhale
jhale added this pull request to the merge queue Mar 6, 2025
Merged via the queue into main with commit 4627a17 Mar 6, 2025
@jhale
jhale deleted the jhale/refactor-la-petsc branch March 6, 2025 19:46
jorgensd added a commit to jorgensd/dolfinx_mpc that referenced this pull request Mar 7, 2025
jorgensd added a commit to jorgensd/dolfinx_mpc that referenced this pull request Mar 7, 2025
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.

4 participants