Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolves #350 -- Adds support for segmenting LinearPipelines #374

Merged
23 commits merged into from
Sep 28, 2022

Conversation

drobison00
Copy link
Contributor

  1. Adds a new API call to LinearPipeline, add_segment_boundary([data_type], [is_shared_pointer]), which will create an egress stage and connected to the most recent linear stage, and an ingress source stage in a new segment. At build, these stages will be connected.
    Known issue here: [BUG] Extension libraries compiled with pybind11 don't have cpptype visibility via get_internals MRC#176 : Currently specifying the data type has no effect and all underlying objects will utilize the default PythonObject path. This is due to the method we were using to match the cpptype of a wrapped python object to the cpptype of an Ingress/EgressPort adapter. Once we're able to add an out of band path for type detection/storage this will be resolve.
  2. Updates Pipeline to be segment aware, and makes the necessary updates to Pipeline internals to handle the changes.
  3. Updates the visualize(..)

@drobison00 drobison00 requested a review from a team as a code owner September 26, 2022 15:44
@drobison00 drobison00 added enhancement Additional functionality added to an existing feature 3 - Ready for Review labels Sep 26, 2022
@drobison00 drobison00 added non-breaking Non-breaking change improvement Improvement to existing functionality labels Sep 26, 2022
Copy link
Contributor

@dagardner-nv dagardner-nv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

I know this really isn't part of your diff... but I noticed that in Pipeline.build we call segment_graph.nodes() a couple of times, maybe we should save this to a local variable.

If we assume the number of nodes in any segment to be smallish and/or if this call is cheap then maybe this isn't important.

morpheus/pipeline/pipeline.py Show resolved Hide resolved
morpheus/pipeline/pipeline.py Outdated Show resolved Hide resolved
drobison00 and others added 4 commits September 27, 2022 12:35
Co-authored-by: David Gardner <96306125+dagardner-nv@users.noreply.github.com>
Remove changes that came in from another MR.
Remove changes from another MR
Copy link
Contributor

@cwharris cwharris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

morpheus/pipeline/linear_pipeline.py Show resolved Hide resolved
morpheus/_lib/src/python_modules/messages.cpp Show resolved Hide resolved
morpheus/pipeline/pipeline.py Show resolved Hide resolved
morpheus/pipeline/pipeline.py Show resolved Hide resolved
@drobison00
Copy link
Contributor Author

@gpucibot merge

@ghost ghost merged commit 94235c6 into nv-morpheus:branch-22.09 Sep 28, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Additional functionality added to an existing feature improvement Improvement to existing functionality non-breaking Non-breaking change
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants