Merged
Conversation
This is to ensure that all MPI routines are called before MPI_Finalise is called. Resolves: gh#3041
The list from gc was incomplete. Using a custom weakref list ensures we can free all objects before MPI_Finalize is called. This reverts 0e4314c, which was not working as it relied on the same, incomplete, list of objects. Resolves: gh#3041
Merged
ZedThree
reviewed
Jan 7, 2025
Comment on lines
+58
to
+63
| def __cinit__(self, {{ init_args}}): | ||
| global _allobjects | ||
| _allobjects.append(weakref.ref(self)) | ||
| {% if not uniquePtr and cobj %} | ||
| self.isSelfOwned = {{ defaultSO }} | ||
| self.cobj = NULL |
Member
There was a problem hiding this comment.
Another option that might be easier to read would be to just template this function
Contributor
Author
There was a problem hiding this comment.
So you mean move this to a separate file?
I do not think it would get much easier.
Certainly the file is a bit long and hard to read, but I am not sure moving this macro to a different file helps a lot with cleaning up.
Member
There was a problem hiding this comment.
Sorry, I meant just have something like:
{% macro class_cinit(init_args="*args, **kwargs", defaultSO=True) %}
cdef c.bool isSelfOwned
cdef object __weakref__
def __cinit__(self, {{ init_args }}):
...
{% endmacro %}and use like:
cdef class Mesh:
...
{{ class_cinit | indent }}Avoids inheritance, reduces the duplication from the __cinit__ method, and keeps the class definitions looking normal
bendudson
approved these changes
Jan 9, 2025
Contributor
Author
|
This is probaly also needed for master ... |
bendudson
added a commit
that referenced
this pull request
Mar 5, 2025
Fix Finalise (#3053 for master)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #3041