A few log level handling cleanups (#6393)

This PR does two things:

1. Centralize `verbose` <-> log level handling

Previously we had conversion to/from the `logger.level_enum` and `verbose` parameters strewn across the codebase. We now centralize this in 2 common utility functions. In the future we should also aim to reduce the number of places where we need to handle this at all, but for now this is a slight improvement.

2. Properly translate `verbose` to `level_enum` in `simpl_set.pyx`

Previously we weren't translating `verbose` to `level_enum` in the `simpl_set` functions. This meant that the default value (`False`) would be cast to `level_enum`, resulting in the logger being set to `0` (trace).

This doesn't fix the larger issue that our `verbose` handling mutates the global logger settings. It does ensure we're at least mutating it to the correct enum value though.

Authors:
  - Jim Crist-Harif (https://github.com/jcrist)

Approvers:
  - Simon Adorf (https://github.com/csadorf)

URL: #6393

python/cuml/cuml/internals/base.pyx

@@ -90,7 +90,7 @@ class VerbosityDescriptor:
     """
     def __get__(self, obj, cls=None):
         if api_context_managers.in_internal_api():
-            return logger.level_enum(6 - obj._verbose)
+            return logger._verbose_to_level(obj._verbose)
         else:
             return obj._verbose
 
@@ -100,7 +100,7 @@ class VerbosityDescriptor:
                 "The log level should always be provided as a level_enum, "
                 "not an integer"
             )
-            obj._verbose = 6 - int(value)
+            obj._verbose = logger._verbose_from_level(value)
         else:
             if isinstance(value, logger.level_enum):
                 raise ValueError(
@@ -268,18 +268,7 @@ class Base(TagsMixin,
         # infrastructure in https://github.com/rapidsai/cuml/pull/6189.
         GlobalSettings().prev_root_cm = GlobalSettings().root_cm
         GlobalSettings().root_cm = True
-        IF GPUBUILD == 1:
-            # Internally, self.verbose follows the spdlog/c++ standard of
-            # 0 is most logging, and logging decreases from there.
-            # So if the user passes an int value for logging, we convert it.
-            if verbose is True:
-                self.verbose = logger.level_enum.debug
-            elif verbose is False:
-                self.verbose = logger.level_enum.info
-            else:
-                self.verbose = logger.level_enum(6 - verbose)
-        ELSE:
-            self.verbose = logger.level_enum(6 - verbose)
+        self.verbose = logger._verbose_to_level(verbose)
         # Please see above note on manipulation of the root_cm. This should be
         # rendered unnecessary with https://github.com/rapidsai/cuml/pull/6189.
         GlobalSettings().root_cm = GlobalSettings().prev_root_cm
@@ -355,7 +344,7 @@ class Base(TagsMixin,
             # VerbosityDescriptor for direct access to the verbose
             # attribute.
             if key == "verbose":
-                var_value = 6 - int(var_value)
+                var_value = logger._verbose_from_level(var_value)
             params[key] = var_value
         return params
 
@@ -375,7 +364,7 @@ class Base(TagsMixin,
             else:
                 # Switch verbose to enum since we are now internal to cuML API
                 if key == "verbose":
-                    value = logger.level_enum(6 - int(value))
+                    value = logger._verbose_to_level(value)
                 setattr(self, key, value)
         return self
 

python/cuml/cuml/internals/logger.pyx

@@ -44,6 +44,21 @@ IF GPUBUILD == 1:
             sys.stdout.flush()
 
 
+def _verbose_to_level(verbose: bool | int) -> level_enum:
+    """Parse the common `verbose` parameter into a `level_enum`."""
+    if verbose is True:
+        return level_enum.debug
+    elif verbose is False:
+        return level_enum.info
+    else:
+        return level_enum(6 - verbose)
+
+
+def _verbose_from_level(level: level_enum) -> int:
+    """Convert a `level_enum` back into an equivalent `verbose` parameter value."""
+    return 6 - int(level)
+
+
 cdef class LogLevelSetter:
     """Internal "context manager" object for restoring previous log level"""
 

python/cuml/cuml/manifold/simpl_set.pyx

@@ -29,6 +29,7 @@ from cuml.manifold.umap_utils import GraphHolder, find_ab_params, \
 
 from cuml.internals.input_utils import input_to_cuml_array, is_array_like
 from cuml.internals.array import CumlArray
+from cuml.internals import logger
 
 from pylibraft.common.handle cimport handle_t
 from pylibraft.common.handle import Handle
@@ -165,7 +166,7 @@ def fuzzy_simplicial_set(X,
         umap_params.p = <float> 2.0
     else:
         umap_params.p = <float> metric_kwds.get("p", 2.0)
-    umap_params.verbosity = verbose
+    umap_params.verbosity = logger._verbose_to_level(verbose)
 
     X_m, _, _, _ = \
         input_to_cuml_array(X,
@@ -366,7 +367,7 @@ def simplicial_set_embedding(
         umap_params.target_metric = MetricType.CATEGORICAL
     umap_params.target_weight = <float> output_metric_kwds['p'] \
         if 'p' in output_metric_kwds else 0.5
-    umap_params.verbosity = verbose
+    umap_params.verbosity = logger._verbose_to_level(verbose)
 
     X_m, _, _, _ = \
         input_to_cuml_array(data,

python/cuml/cuml/manifold/umap.pyx

@@ -444,28 +444,15 @@ class UMAP(UniversalBase,
         self._input_hash = None
         self._small_data = False
 
-        self.precomputed_knn = extract_knn_infos(precomputed_knn,
-                                                 n_neighbors)
-
-        # We need to set this log level here so that it is propagated in time
-        # for the logger.info call below. We cannot use the verbose parameter
-        # directly because Base.__init__ contains the logic for converting
-        # boolean values to suitable integers. We access self._verbose instead
-        # of self.verbose because due to the same issues described in
-        # Base.__init__'s logic for setting verbose, this code is not
-        # considered to be within a root context and therefore considered
-        # external. Rather than mucking with the decorator, for this specific
-        # case since we're trying to set the properties of the underlying
-        # logger we may as well access our underlying value directly and
-        # perform the necessary arithmetic.
-        logger.set_level(logger.level_enum(6 - self._verbose))
+        self.precomputed_knn = extract_knn_infos(precomputed_knn, n_neighbors)
 
         if build_algo == "auto" or build_algo == "brute_force_knn" or build_algo == "nn_descent":
             if self.deterministic and build_algo == "auto":
                 # TODO: for now, users should be able to see the same results as previous version
                 # (i.e. running brute force knn) when they explicitly pass random_state
                 # https://github.com/rapidsai/cuml/issues/5985
-                logger.info("build_algo set to brute_force_knn because random_state is given")
+                with logger.set_level(logger._verbose_to_level(verbose)):
+                    logger.info("build_algo set to brute_force_knn because random_state is given")
                 self.build_algo ="brute_force_knn"
             else:
                 self.build_algo = build_algo

python/cuml/cuml/svm/linear.pyx

@@ -31,7 +31,7 @@ from cuml.common.array_descriptor import CumlArrayDescriptor
 from cuml.internals.array import CumlArray
 from cuml.internals.base import Base
 from cuml.internals.logger cimport level_enum
-from cuml.internals.logger import level_enum as py_level_enum
+from cuml.internals import logger
 from pylibraft.common.handle cimport handle_t
 from pylibraft.common.interruptible import cuda_interruptible
 from cuml.common import input_to_cuml_array
@@ -208,15 +208,11 @@ class LSVMPWrapper(LSVMPWrapper_):
 
     @property
     def verbose(self):
-        # Reverse ordering of log levels to convert spdlog level values to
-        # Scikit-Learn log level values
-        return 6 - int(self._getparam('verbose'))
+        return logger._verbose_from_level(self._getparams('verbose'))
 
     @verbose.setter
     def verbose(self, level: int):
-        # Reverse ordering of log levels to convert spdlog level values to
-        # Scikit-Learn log level values
-        self._setparam('verbose', py_level_enum(6 - level))
+        self._setparam('verbose', logger._verbose_to_level(level))
 
 
 # Add properties for parameters with a trivial conversion

python/cuml/cuml/tests/test_logger.py

@@ -13,11 +13,32 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-
 from contextlib import redirect_stdout
-import cuml.internals.logger as logger
 from io import StringIO, TextIOWrapper, BytesIO
 
+import pytest
+
+import cuml.internals.logger as logger
+
+
+@pytest.mark.parametrize(
+    "verbose, level, verbose_numeric",
+    [
+        (False, logger.level_enum.info, 4),
+        (True, logger.level_enum.debug, 5),
+        (0, logger.level_enum.off, 0),
+        (1, logger.level_enum.critical, 1),
+        (2, logger.level_enum.error, 2),
+        (3, logger.level_enum.warn, 3),
+        (4, logger.level_enum.info, 4),
+        (5, logger.level_enum.debug, 5),
+        (6, logger.level_enum.trace, 6),
+    ],
+)
+def test_verbose_to_from_level(verbose, level, verbose_numeric):
+    assert logger._verbose_to_level(verbose) == level
+    assert logger._verbose_from_level(level) == verbose_numeric
+
 
 def test_logger():
     logger.trace("This is a trace message")