Skip to content

Conversation

@connorjward
Copy link
Collaborator

The current design has created a nasty dependency cycle between Firedrake and ngsPETSc that makes making Firedrake releases more challenging.

Here I try to remove all Firedrake imports from ngsPETSc. Some of the utils/firedrake folder still exists because it didn't actually need Firedrake.

There is definitely discussion to be had about where the line is drawn between ngsPETSc and Firedrake, but the critical thing is ngsPETSc should not be importing Firedrake.

@connorjward
Copy link
Collaborator Author

@UZerbinati what are your thoughts? For us solving this cycle is important but not yet critical.

What should live in ngsPETSc and what should live in Firedrake?

@stefanozampini
Copy link
Collaborator

This minimally overlaps with #92. I believe the right thing to do is to move the code to support netgen meshes to Firedrake as proposed here. This will remove the horrific setattr(fd.MeshGeometry,..) hack we currently use

@connorjward
Copy link
Collaborator Author

This minimally overlaps with #92. I believe the right thing to do is to move the code to support netgen meshes to Firedrake as proposed here. This will remove the horrific setattr(fd.MeshGeometry,..) hack we currently use

Yeah it's quite critical for Firedrake that this happens. There are a couple of upcoming API changes that will require some ngsPETSc fixes to work with Firedrake main, and hence break release.

Here I have moved all the code that explicitly uses Firedrake into Firedrake but left some of utils/firedrake/ because it doesn't actually import Firedrake. I wonder if that's the right approach. We could just ingest the entire directory. I have been waiting on some feedback from ngsPETSc on what they are happy with.

@UZerbinati
Copy link
Collaborator

Hey ! I've been waiting on giving feedback on this because we had major new features related to Netgen coming in this month (hopefully). And I thought it would have been more clear if first we get everything we aim to get merged in Netgen merged and then move what we feel should be moved to Firedrake.
For example to speed up the snapping to grid in the mesh hierarchy and some other loop in the mesh generation we are using NUMBA which would add a dependencies that might not be ideal in Firedrake and so on.
Maybe we can discuss it in person on Tuesday?

@connorjward
Copy link
Collaborator Author

Hey ! I've been waiting on giving feedback on this because we had major new features related to Netgen coming in this month (hopefully). And I thought it would have been more clear if first we get everything we aim to get merged in Netgen merged and then move what we feel should be moved to Firedrake. For example to speed up the snapping to grid in the mesh hierarchy and some other loop in the mesh generation we are using NUMBA which would add a dependencies that might not be ideal in Firedrake and so on. Maybe we can discuss it in person on Tuesday?

Happy to discuss in person.

I'm don't mind waiting a couple of weeks but any longer is likely to start causing issues.

I don't think adding numba is an issue. We can make it an optional dependency.

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.

3 participants