Generalize StructArrays to ContainerArrays and refactor View class structure #1504
+494
−141
Conversation
This file contains 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
alexnick83
approved these changes
Jan 15, 2024
| @@ -516,13 +510,15 @@ def _construct_args(self, kwargs) -> Tuple[Tuple[Any], Tuple[Any]]: | |||
| if atype.optional is False: # If array cannot be None | |||
| raise TypeError(f'Passing a None value to a non-optional array in argument "{a}"') | |||
| # Otherwise, None values are passed as null pointers below | |||
| elif isinstance(arg, ctypes._Pointer): | |||
There was a problem hiding this comment.
Could we leave a note here about the use of an underscored type? I am mostly concerned about the developers changing or removing it in a future version.
There was a problem hiding this comment.
Interestingly, it is part of the Python stable API: https://docs.python.org/3/library/ctypes.html#ctypes._Pointer
It is also the only one we can check against that covers all ctypes.POINTER types.
This PR refactors the notion of views to a `View` interface, and provides views to arrays, structures, and container arrays. It also adds a syntactic-sugar/helper API to define a view of an existing data descriptor. Depends on merging #1504
github-merge-queue
bot
removed this pull request from the merge queue due to a conflict with the base branch
tbennun
changed the title
github-merge-queue bot
pushed a commit
that referenced
this pull request
May 3, 2024
The refactoring of Views in PR #1504 led to the creation of the ArrayView type. This PR addresses an issue in the Python ProgramVisitor, where ArrayViews are not recognized properly as Views (of Arrays), leading to a NotImplementedError. The fix is simple: when checking if a container is an Array or a View (of an Array), instead of making a direct equality comparison to Array or View, a subclass comparison against Array is performed. The latter returns true if the container is an Array or any Array subclass, including ArrayViews.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR enables the use of an array data descriptor that contains a nested data descriptor (e.g., ContainerArray of Arrays). Its contents can then be viewed normally with View or StructureView.
With this, concepts such as jagged arrays are natively supported in DaCe (see test for example).
Also adds support for using ctypes pointers and arrays as arguments to SDFGs.
This PR also refactors the notion of views to a View interface, and provides views to arrays, structures, and container arrays. It also adds a syntactic-sugar/helper API to define a view of an existing data descriptor.