Skip to content

Commit

Permalink
Fix config validation failures caused by NVTX pipeline wrappers (#11460)
Browse files Browse the repository at this point in the history
* Enable Cython<->Python bindings for `Pipe` and `TrainablePipe` methods

* `pipes_with_nvtx_range`: Skip hooking methods whose signature cannot 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.
  • Loading branch information
shadeMe authored Sep 12, 2022
1 parent 6b83fee commit 0ec9a69
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 4 deletions.
7 changes: 5 additions & 2 deletions spacy/ml/callbacks.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,14 @@ def pipes_with_nvtx_range(
types.MethodType(nvtx_range_wrapper_for_pipe_method, pipe), func
)

# Try to preserve the original function signature.
# We need to preserve the original function signature so that
# the original parameters are passed to pydantic for validation downstream.
try:
wrapped_func.__signature__ = inspect.signature(func) # type: ignore
except:
pass
# Can fail for Cython methods that do not have bindings.
warnings.warn(Warnings.W122.format(method=name, pipe=pipe.name))
continue

try:
setattr(
Expand Down
2 changes: 1 addition & 1 deletion spacy/pipeline/pipe.pyx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# cython: infer_types=True, profile=True
# cython: infer_types=True, profile=True, binding=True
from typing import Optional, Tuple, Iterable, Iterator, Callable, Union, Dict
import srsly
import warnings
Expand Down
2 changes: 1 addition & 1 deletion spacy/pipeline/trainable_pipe.pyx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# cython: infer_types=True, profile=True
# cython: infer_types=True, profile=True, binding=True
from typing import Iterable, Iterator, Optional, Dict, Tuple, Callable
import srsly
from thinc.api import set_dropout_rate, Model, Optimizer
Expand Down

0 comments on commit 0ec9a69

Please sign in to comment.