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

Fix config validation failures caused by NVTX pipeline wrappers #11460

Merged
merged 2 commits into from
Sep 12, 2022

Conversation

shadeMe
Copy link
Contributor

@shadeMe shadeMe commented Sep 8, 2022

Description

When loading pipelines from a config file, the arguments passed to individual pipeline components are validated by pydantic during init. For this, the validation model attempts to parse the function signature of the component's c'tor/entry point in order to check if all mandatory parameters are present in the config file.

When using models_and_pipes_with_nvtx_range as an after_pipeline_creation callback, the methods of all pipeline components get replaced by an NVTX range wrapper before the above-mentioned validation takes place. This can be problematic for components that are implemented as Cython extension types - if the extension type is not compiled with Python bindings for its methods, they will have no signatures at runtime. This resulted in pydantic validating the wrapper's parameters against those in the config, which raised errors.

To avoid this, we no longer apply the wrapper to (Cython) methods that do not have signatures.

This PR also enables bindings for both the Pipe and TrainablePipe Cython classes, thereby adding support for methods that aren't overridden by their subclasses.

Types of change

Bug fix

Checklist

  • I confirm that I have the right to submit this contribution under the project's MIT license.
  • I ran the tests, and all new and existing tests passed.
  • My changes don't require a change to the documentation, or if they do, I've added all required information.

…be ascertained

When loading pipelines from a config file, the arguments passed to individual pipeline components is validated by `pydantic` during init. For this, the validation model attempts to parse the function signature of the component's c'tor/entry point so that it can check if all mandatory parameters are present in the config file.

When using the `models_and_pipes_with_nvtx_range` as a `after_pipeline_creation` callback, the methods of all pipeline components get replaced by a NVTX range wrapper **before** the above-mentioned validation takes place. This can be problematic for components that are implemented as Cython extension types - if the extension type is not compiled with Python bindings for its methods, they will have no signatures at runtime. This resulted in `pydantic` matching the *wrapper's* parameters with the those in the config and raising errors.

To avoid this, we now skip applying the wrapper to any (Cython) methods that do not have signatures.
@shadeMe shadeMe added bug Bugs and behaviour differing from documentation feat / pipeline Feature: Processing pipeline and components labels Sep 8, 2022
@shadeMe shadeMe changed the title Fix config validation failures caused by NVTX range pipeline wrappers Fix config validation failures caused by NVTX pipeline wrappers Sep 8, 2022
@svlandeg svlandeg added the feat / ux Feature: User experience, error messages etc. label Sep 12, 2022
@svlandeg svlandeg merged commit 0ec9a69 into explosion:master Sep 12, 2022
@shadeMe shadeMe deleted the fix/nvtx-pipeline-callback branch September 12, 2022 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs and behaviour differing from documentation feat / pipeline Feature: Processing pipeline and components feat / ux Feature: User experience, error messages etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants