diff --git a/petsctools/log.py b/petsctools/log.py new file mode 100644 index 0000000..7a95853 --- /dev/null +++ b/petsctools/log.py @@ -0,0 +1,10 @@ +import logging + + +LOGGER = logging.getLogger("petsctools") + +debug = LOGGER.debug +info = LOGGER.info +warning = LOGGER.warning +error = LOGGER.error +critical = LOGGER.critical diff --git a/petsctools/options.py b/petsctools/options.py index 20d17a0..9cf34a2 100644 --- a/petsctools/options.py +++ b/petsctools/options.py @@ -2,7 +2,6 @@ import contextlib import functools -import itertools import warnings import weakref from collections.abc import Iterable @@ -11,6 +10,7 @@ import petsc4py +import petsctools.log from petsctools.appctx import AppContextManager from petsctools.exceptions import ( PetscToolsException, @@ -18,6 +18,7 @@ PetscToolsWarning, ) + _commandline_options = None @@ -372,18 +373,18 @@ class OptionsManager: parameters The dictionary of parameters to use. options_prefix - The prefix to look up items in the global options database - (may be ``None``, in which case only entries from ``parameters`` - will be considered. - If no trailing underscore is provided, one is appended. Hence - ``foo_`` and ``foo`` are treated equivalently. As an exception, - if the prefix is the empty string, no underscore is appended. + The prefix to look up items in the global options database. If `None` + then the prefix is generated from ``default_prefix``. If no trailing + underscore is provided, one is appended. Hence ``foo_`` and ``foo`` + are treated equivalently. As an exception, if the prefix is the empty + string, no underscore is appended. default_prefix The base string to generate default prefixes. If options_prefix is not provided then a prefix is automatically generated with the form "{default_prefix}_{n}", where n is a unique integer. Note that - because the unique integer is not stable any options passed via the - command line with a matching prefix will be ignored. + because the unique integer is not stable the auto-generated prefix for + a particular solver may change between Python invocations due to + changes elsewhere in the code. default_options_set The prefix set for any default shared with other solvers. See :class:`DefaultOptionSet` for more information. @@ -403,14 +404,13 @@ class OptionsManager: AppContextManager """ - count = itertools.count() + count = 0 def __init__(self, parameters: dict, options_prefix: str | None = None, default_prefix: str | None = None, default_options_set: DefaultOptionSet | None = None, appmngr: AppContextManager | None = None): - super().__init__() if parameters is None: parameters = {} else: @@ -418,61 +418,61 @@ def __init__(self, parameters: dict, parameters = flatten_parameters(parameters) # If no prefix is provided generate a default prefix - # and ignore any command line options if options_prefix is None: default_prefix = default_prefix or "petsctools_" default_prefix = _validate_prefix(default_prefix) - self.options_prefix = f"{default_prefix}{next(self.count)}_" - self.parameters = parameters - self.to_delete = set(parameters) - + options_prefix = f"{default_prefix}{self.count}_" + self.count += 1 + unsafe_prefix = True else: options_prefix = _validate_prefix(options_prefix) - self.options_prefix = options_prefix - - # Are we part of a solver set sharing defaults? - if default_options_set: - if options_prefix not in default_options_set.custom_prefixes: - raise ValueError( - f"The options_prefix {options_prefix} must be one" - f" of the custom_prefixes of the DefaultOptionSet" - f" {default_options_set.custom_prefixes}") - default_options = get_default_options( - default_options_set, self.options_object) - else: - default_options = {} - - # Note: we need to know which parameters to_delete - # so we need to exclude the relevant command line - # options when combining the parameters from the - # defaults and the source code. - - # Start building parameters from the defaults so - # that they will overwritten by any other source. - self.parameters = { - k: v - for k, v in default_options.items() - if options_prefix + k not in get_commandline_options() - } - - # Update using the parameters passed in the code but - # exclude those options from the dict that were passed - # on the commandline because those have global scope and are - # not under the control of the options manager. - self.parameters.update({ - k: v - for k, v in parameters.items() - if options_prefix + k not in get_commandline_options() - }) - self.to_delete = set(self.parameters) - - # Now update parameters from options, so that they're - # available to solver setup (for, e.g., matrix-free). - # Can't ask for the prefixed guy in the options object, - # since that does not DTRT for flag options. - for k, v in self.options_object.getAll().items(): - if k.startswith(self.options_prefix): - self.parameters[k[len(self.options_prefix):]] = v + unsafe_prefix = False + + # Are we part of a solver set sharing defaults? + if default_options_set: + if options_prefix not in default_options_set.custom_prefixes: + raise ValueError( + f"The options_prefix {options_prefix} must be one" + f" of the custom_prefixes of the DefaultOptionSet" + f" {default_options_set.custom_prefixes}") + default_options = get_default_options( + default_options_set, self.options_object) + else: + default_options = {} + + # The parameters to drop from the global options when we leave the + # inserted_options context. This is everything except for options + # passed on the command line. + to_delete = set(parameters.keys()) + + # Start building parameters from the defaults so + # that they will overwritten by any other source. + parameters = default_options | parameters + unsafe_options = [] + for full_key, v in self.options_object.getAll().items(): + if full_key.startswith(options_prefix): + if unsafe_prefix: + unsafe_options.append(full_key) + + key = full_key[len(options_prefix):] + parameters[key] = v + + if key in to_delete: + # option is set globally, don't drop when we exit the + # context manager + to_delete.remove(key) + if unsafe_options: + unsafe_options_str = "\n".join(( + f" {opt}" for opt in unsafe_options + )) + petsctools.log.warning(f"""\ +Setting options using an autogenerated prefix '{options_prefix}' is unsafe: +{unsafe_options_str}""" + ) + + self.parameters = parameters + self.to_delete = to_delete + self.options_prefix = options_prefix self._setfromoptions = False @@ -557,15 +557,18 @@ def inserted_options(self): else: yield finally: - for k in self.to_delete: + for k in self.parameters: if self.options_object.used(self.options_prefix + k): self._used_options.add(k) + for k in self.to_delete: del self.options_object[self.options_prefix + k] @functools.cached_property def options_object(self): from petsc4py import PETSc + # We can't pass the prefix here because that doesn't DTRT + # for flag options return PETSc.Options() diff --git a/tests/test_options.py b/tests/test_options.py index bea4761..9dabf35 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -161,6 +161,57 @@ def test_default_options(): assert options2.parameters["opt4"] == "6" +@pytest.mark.skipnopetsc4py +@pytest.mark.parametrize("options_prefix", (None, "", "custom_")) +def test_commandline_options(caplog, options_prefix): + from petsc4py import PETSc + + if options_prefix is None: + true_prefix = f"petsctools_{petsctools.OptionsManager.count}_" + else: + true_prefix = options_prefix + + # Put some options in the database as though they were passed by a user on + # the command line + options = PETSc.Options() + options["opt1"] = "unused" + options[f"{true_prefix}opt2"] = "will_overwrite" + options[f"{true_prefix}opt3"] = "extra" + + default_params = { + # this will get ignored because we pass something on the command line + "opt2": "default_opt2", + # this will be inserted and popped from the database + "opt4": "default_opt4", + } + om = petsctools.OptionsManager( + default_params, options_prefix=options_prefix + ) + assert om.options_prefix == true_prefix, \ + "The later tests are invalid if these prefixes do not match" + + with om.inserted_options(): + assert options["opt1"] == "unused" + assert options[f"{om.options_prefix}opt2"] == "will_overwrite" + assert options[f"{om.options_prefix}opt3"] == "extra" + assert options[f"{om.options_prefix}opt4"] == "default_opt4" + + if options_prefix is None: + assert len(caplog.records) == 1, "Expected a warning here" + assert caplog.messages[0].startswith( + "Setting options using an autogenerated prefix" + ) + else: + assert not caplog.records, "Nothing should be logged" + + # Make sure the command line options are persistent but the options added + # by the options manager go away. + assert options["opt1"] == "unused" + assert options[f"{om.options_prefix}opt2"] == "will_overwrite" + assert options[f"{om.options_prefix}opt3"] == "extra" + assert f"{om.options_prefix}opt4" not in options + + @pytest.mark.skipnopetsc4py def test_inserted_options_dict(): from petsc4py import PETSc