Skip to content

Python PETSc Example BP1 #1806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tylercollins5737
Copy link

@tylercollins5737 tylercollins5737 commented Apr 19, 2025

Hello!

This is a Python implementation of bpsraw.c for BP1 and was initially discussed in #1774 . I'm aiming to recreate the benchmark behavior using libceed and petsc4py. I appreciate any feedback or indications I'm headed in the right direction!

It does run, but this is a work in progress. Remaining tasks:

  • fix the matrix operations to work like the MatShell C implementation
  • get PETSc Vec scatter working
  • QFunction
  • Error function
  • add meaningful comments
  • improve the printed output metrics
  • create a test

Comment on lines 102 to 115
matrix = PETSc.Mat().createAIJ([num_dofs, num_dofs], comm=PETSc.COMM_WORLD)
matrix.setFromOptions()
matrix.setUp()
for i in range(num_dofs):
e_vec = ceed.Vector(num_dofs)
r_vec = ceed.Vector(num_dofs)
e_vec.set_value(0.0)
r_vec.set_value(0.0)
with e_vec.array_write() as arr:
arr[i] = 1.0
op.apply(e_vec, r_vec)
with r_vec.array_read() as r_arr:
matrix.setValues(range(num_dofs), [i], r_arr)
matrix.assemble()
Copy link
Member

Choose a reason for hiding this comment

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

You'll want a MatShell, not a MatAIJ

Copy link
Author

Choose a reason for hiding this comment

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

Apologies for leaving this a bit stale. Addressing this led to a lot of other changes. I had trouble setting the matrix type to "shell" and applying context, kept getting segmentation faults. So, instead of using the "shell" type, I used the "python" type and set the context through a python class. Pulled that implementation from the petsc4py 2DPoisson example (about half way down) --> link. I'll tidy up what I have a submit an update soon.

Copy link
Member

Choose a reason for hiding this comment

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

Overall this is looking like the right approach. The big thing is to try to avoid copying around data during frequent operations, like the matrix application

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for all the suggestions! Definitely, keeping the data in place would mean less overhead and better efficiency

@valeriabarra
Copy link
Contributor

@tylercollins5737 , thank you for your work. Are you able to address the reviewer's comments?

@tylercollins5737
Copy link
Author

@valeriabarra I was able to get it working using the "python" matrix type, MatAIJ was not the best. I believe the "python" type and MatCreateShell can achieve the same thing.

Comment on lines +93 to +95
with self.v.array_read() as va:
y.setValues(range(lo, hi), va[lo:hi])
y.assemble()
Copy link
Member

Choose a reason for hiding this comment

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

Ideally, you'd set v to use y's array before you apply the libCEED operator

Copy link
Author

Choose a reason for hiding this comment

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

That makes more sense, thank you!

Comment on lines +39 to +40
def CreateRestriction(ceed, mesh_elem, P, num_comp, ndofs, offsets):
return ceed.ElemRestriction(mesh_elem, P, num_comp, 1, ndofs, offsets, cmode=libceed.USE_POINTER)
Copy link
Member

Choose a reason for hiding this comment

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

If this just wraps another function call, I think its clearer to use the function call itself

Copy link
Author

Choose a reason for hiding this comment

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

Totally agree, I have another CreateRestriction function that reflects C but its a WIP

Co-authored-by: Jeremy L Thompson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants