Add ability to specify port data type for known c++ types from Python #153
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
drobison00
added
improvement
drobison00
changed the title
mdemoret-nv
requested changes
Jul 30, 2022
mdemoret-nv
approved these changes
Aug 3, 2022
|
@gpucibot merge |
ghost
merged commit
ghost
pushed a commit
that referenced
this pull request
Sep 6, 2022
…tructor (#165) If you create a source or a sink class that derives from `PythonSink`, `PythonSource` or `PythonNode` and the message type does not have a default constructor, you will run into a compilation error. This was due to the fact that the new Port auto registration added in PR #153 tries to compile ingress/egress ports for both `T` and `shared_ptr<T>`. This PR adds a check to skip compiling a port with `T` if `T` does not have a default constructor. Authors: - Michael Demoret (https://github.com/mdemoret-nv) Approvers: - Devin Robison (https://github.com/drobison00) URL: #165
This pull request was closed.
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.
TODO: still need to add additional unit tests, and update to support a 3rd tuple parameter indicating if the raw data type or shared pointer should be used as the underlying node data type.
This will make a number of non-breaking changes to the process of Ingress and Egress port creation via Python.
Implements @mdemoret-nv 's changes to remove the need for specifying Ingress and Egress port data types during construction, and instead shifts to a system of providing lambda functions that create the appropriate derived object and return a common un-templated base; this results in a much more flexible, cleaner, interface and eliminates any hard cap on the number of ports that can be created on a segment from Python.
Extends previous updates to support python bindings for ingress/egress port creation to support either simple strings, or for specifying a string + pybind11 registered type tuple.
pipeline.make_segment("name", ["a", "b", "c"], [], init)pipleine.make_segment("name", [("a", my_type), ("b", another_type), "c"]Internally, the "simple string" case, will result in Ingress/Egress nodes with
PyHolderdate types; which can be inefficient, and require multiple edge converters when connected to pure c++ (non-python) data types. With the newly supported syntax, providing a pybind11 registered type will result in an attempt to lookup the associated (wrapped) cpptype and create an Ingress or Egress port using that type.PortUtilclass. The port registry stores a map from std::type_index to PortUtil, and PortUtil provides a number of helper functions for creating, casting, and manipulating Ingress and Egress ports.