Skip to content

Updates for upstream PETSc assembly changes - #47

Merged
francesco-ballarin merged 14 commits into
mainfrom
update
Jul 13, 2025
Merged

Updates for upstream PETSc assembly changes#47
francesco-ballarin merged 14 commits into
mainfrom
update

Conversation

@francesco-ballarin

@francesco-ballarin francesco-ballarin commented Jul 4, 2025

Copy link
Copy Markdown
Member

I am finally working on bringing multiphenicsx up to date with changes on dolfinx main branch related to PETSc assembly.

The main upstream changes are (will tick the box once I've worked on that):

For now I've worked on the first two items, and made sure that the tests pass.
I'll update the tutorials after working on the last two items, therefore CI is still failing for now.

cc @ordinary-slim in case you want to have a look, test it on your codes or review it.

@francesco-ballarin
francesco-ballarin force-pushed the update branch 7 times, most recently from 5aba249 to 9a38c1d Compare July 6, 2025 17:41
@ordinary-slim

ordinary-slim commented Jul 6, 2025

Copy link
Copy Markdown
Collaborator

My tests do work after the API changes. Note that the old API used to call create_vector(form, restriction); this still runs, but with restriction=Noneand kind=restriction, so it produces an unrestricted vector. I don't think this needs to be addressed.

@francesco-ballarin

Copy link
Copy Markdown
Member Author

My tests do work after the API changes.

Great, thanks for checking. I'll merge this as soon as CI is green.

Note that the old API used to call create_vector(form, restriction); this still runs, but with restriction=Noneand kind=restriction, so it produces an unrestricted vector. I don't think this needs to be addressed.

I do understand your point, but there is nothing I can make about this. The rationale is that we should copy the signature from dolfinx, and add the restriction arguments at the end. A fix for this specific API change would be to swap kind and restriction, but doing so would make that function inconsistent with the rationale used in the others. Still, I don't think it's worth doing this, considering that there are much major API changes and the users will need to review again their entire code...

@francesco-ballarin

Copy link
Copy Markdown
Member Author

Note that the old API used to call create_vector(form, restriction); this still runs, but with restriction=Noneand kind=restriction, so it produces an unrestricted vector. I don't think this needs to be addressed.

I do understand your point, but there is nothing I can make about this. The rationale is that we should copy the signature from dolfinx, and add the restriction arguments at the end. A fix for this specific API change would be to swap kind and restriction, but doing so would make that function inconsistent with the rationale used in the others. Still, I don't think it's worth doing this, considering that there are much major API changes and the users will need to review again their entire code...

I've pushed a commit which adds an assert for this case, which hopefully should be enough for the user to realize that there has been an API change.

@ordinary-slim

Copy link
Copy Markdown
Collaborator

Code looks good to me. assign function in petsc submodule is a very welcome addition.

…aliziation used to work with pyvista 0.44.*, but stopped with working 0.45.0
@francesco-ballarin

Copy link
Copy Markdown
Member Author

Will delay merging until FEniCS/dolfinx#3785 is merged upstream, because that fixes on failed assertion in the tutorials.

@francesco-ballarin
francesco-ballarin force-pushed the update branch 2 times, most recently from e958d0b to c65d3a3 Compare July 13, 2025 08:43
@francesco-ballarin
francesco-ballarin merged commit 218ce32 into main Jul 13, 2025
6 checks passed
@francesco-ballarin
francesco-ballarin deleted the update branch July 13, 2025 09:18
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