Skip to content

Destroy object the PETSc garbage collector would have issue with#5235

Open
UZerbinati wants to merge 1 commit into
mainfrom
uz/netgen_garbage_collector
Open

Destroy object the PETSc garbage collector would have issue with#5235
UZerbinati wants to merge 1 commit into
mainfrom
uz/netgen_garbage_collector

Conversation

@UZerbinati

Copy link
Copy Markdown
Contributor

PETSc Garbage collector was hanging on my machine when running test locally :)

@UZerbinati
UZerbinati requested a review from connorjward July 10, 2026 15:11
@pbrubeck

Copy link
Copy Markdown
Contributor

Please add parallel tests

@connorjward

Copy link
Copy Markdown
Contributor

This shouldn't be hanging... the garbage collector should never do this

@UZerbinati

Copy link
Copy Markdown
Contributor Author

Please add parallel tests

What do you mean ? ngsPETSc and netgen interface is already tested in parallel, no ?

@pbrubeck

pbrubeck commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What do you mean ? ngsPETSc and netgen interface is already tested in parallel, no ?

This does not seem to be the case for the adaptive refinement code in Netgen. However we're going to replace this ad-hoc renumbering hell with DMAdaptLabel in #5215

@connorjward

Copy link
Copy Markdown
Contributor

Was this happening reliably or stochastically? Others have also random observed garbage collector hangs and I would like to resolve it.

@pbrubeck

Copy link
Copy Markdown
Contributor

Was this happening reliably or stochastically? Others have also random observed garbage collector hangs and I would like to resolve it.

How old are those reports? I fixed a very common hanging issue that was triggered by repetedly calling multigrid not too long ago https://gitlab.com/petsc/petsc/-/merge_requests/8694

@UZerbinati

Copy link
Copy Markdown
Contributor Author

What do you mean ? ngsPETSc and netgen interface is already tested in parallel, no ?

This does not seem to be the case for the adaptive refinement code in Netgen. However we're going to replace this ad-hoc renumbering hell with DMAdaptLabel in #5215

Looking forward to that :)

@UZerbinati

Copy link
Copy Markdown
Contributor Author

Was this happening reliably or stochastically? Others have also random observed garbage collector hangs and I would like to resolve it.

Happened consistently on my machine and on @ThighamOxford machine consistently. Everything was up to date current main.

@connorjward

Copy link
Copy Markdown
Contributor

Was this happening reliably or stochastically? Others have also random observed garbage collector hangs and I would like to resolve it.

Happened consistently on my machine and on @ThighamOxford machine consistently. Everything was up to date current main.

That's awesome. If I find the time I really need to dig into what's causing this.

@UZerbinati

Copy link
Copy Markdown
Contributor Author

I still believe is not a bad idea to destroy stuff we will not use anyway, no ? Or is this not standard practice in Firedrake ?

@connorjward

Copy link
Copy Markdown
Contributor

I still believe is not a bad idea to destroy stuff we will not use anyway, no ? Or is this not standard practice in Firedrake ?

In theory it's a completely pointless thing to do because Python will destroy things for us. We try to avoid C-style coding in favour of being Pythonic. This should be added to our style guide.

However, it doesn't do any harm and is clearly exposing an issue with the garbage collector. I am happy to merge this if @pbrubeck is.

@pbrubeck

Copy link
Copy Markdown
Contributor

I still believe is not a bad idea to destroy stuff we will not use anyway, no ? Or is this not standard practice in Firedrake ?

In theory it's a completely pointless thing to do because Python will destroy things for us. We try to avoid C-style coding in favour of being Pythonic. This should be added to our style guide.

However, it doesn't do any harm and is clearly exposing an issue with the garbage collector. I am happy to merge this if @pbrubeck is.

I think we should give it another go with Claude with this in mind, it could be very helpful to get to the bottom of this.

Comment thread firedrake/mesh.py
section0, mvec0 = self.topology_dm.distributeField(sfBCInv,
self._cell_numbering,
mvec)
mark_np = mvec0.getArray().copy()

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.

Maybe this copy is all we needed?

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