Skip to content

Commit

Permalink
Make sure __init__ is called in graph callback. (#3881)
Browse files Browse the repository at this point in the history
I made the mistake and got a segmentation fault.  A value error might be nicer.

Authors:
  - Jiaming Yuan (https://github.com/trivialfis)

Approvers:
  - Corey J. Nolet (https://github.com/cjnolet)

URL: #3881
  • Loading branch information
trivialfis committed May 20, 2021
1 parent 0b33f9d commit b7a634a
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 5 deletions.
6 changes: 3 additions & 3 deletions python/cuml/internals/callbacks_implems.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, NVIDIA CORPORATION.
* Copyright (c) 2019-2021, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -71,8 +71,8 @@ namespace ML {
}

public:
PyObject* pyCallbackClass;
PyObject* pyCallbackClass = nullptr;
};

}
}
}
6 changes: 5 additions & 1 deletion python/cuml/internals/internals.pyx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#
# Copyright (c) 2019, NVIDIA CORPORATION.
# Copyright (c) 2019-2021, NVIDIA CORPORATION.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -79,4 +79,8 @@ cdef class GraphBasedDimRedCallback(PyCallback):
self.native_callback.pyCallbackClass = <PyObject *><void*>self

def get_native_callback(self):
if self.native_callback.pyCallbackClass == NULL:
raise ValueError(
"You need to call `super().__init__` in your callback."
)
return <uintptr_t>&(self.native_callback)
13 changes: 12 additions & 1 deletion python/cuml/test/test_internals_api.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2018-2019, NVIDIA CORPORATION.
# Copyright (c) 2018-2021, NVIDIA CORPORATION.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand All @@ -24,6 +24,10 @@
class CustomCallback(GraphBasedDimRedCallback):
preprocess_event, epoch_event, train_event = False, 0, False

def __init__(self, skip_init=False):
if not skip_init:
super().__init__()

def check(self):
assert(self.preprocess_event)
assert(self.epoch_event > 10)
Expand All @@ -45,3 +49,10 @@ def test_internals_api(n_components):
reducer = UMAP(n_components=n_components, callback=callback)
reducer.fit(data)
callback.check()

# Make sure super().__init__ is called
callback = CustomCallback(skip_init=True)
model = UMAP(n_epochs=10, callback=callback)

with pytest.raises(ValueError):
model.fit_transform(data)

0 comments on commit b7a634a

Please sign in to comment.