Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions docs/changes/newsfragments/8523.new
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
Instruments can now opt in to a logger of their own, rather than sharing a
single logger with every other instrument in the process. Setting the class
attribute ``default_logger_scope`` to ``"instrument"`` makes ``Instrument.log``
(and ``VisaInstrument.visa_log``) use a logger named after the driver class and
the instrument's ``name_parts``, so that levels, handlers and ``propagate`` can
be configured per instrument::

class MyDriver(VisaInstrument):
default_logger_scope = "instrument"

The scoped loggers mirror the instrument hierarchy, for example
``qcodes.instrument.instrument_base.MyDriver`` for the driver,
``qcodes.instrument.instrument_base.MyDriver.myinst`` for one of its
instruments and ``qcodes.instrument.instrument_base.MyDriver.myinst.ChanA`` for
one of that instrument's channels. A level can therefore be set for a whole
driver class, a single instrument or a single channel, and is inherited by
everything below it. Setting the level for a whole driver also works before any
of its instruments exist, so it can be configured via ``logger_levels`` in
``qcodesrc.json``.

The default is unchanged: without opting in, all instruments keep sharing one
logger exactly as before.
102 changes: 102 additions & 0 deletions docs/examples/logging/logging_example.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,108 @@
" driver.cartesian((0, 0, 1))"
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"## One logger per instrument\n",
"By default all instruments share a single logger, named `qcodes.instrument.instrument_base` (and `...instrument_base.com.visa` for the VISA communication). This means that calling `setLevel`, `addHandler` or setting `propagate` on `instrument.log.logger` affects every other instrument in the process as well.\n",
"\n",
"A driver can opt in to a logger of its own by setting `default_logger_scope` to `\"instrument\"`. This can be done on the driver class, which also works for drivers you do not control:"
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"class ScopedAMIModel430(AMIModel430):\n",
" default_logger_scope = \"instrument\"\n",
"\n",
"\n",
"mag_w = ScopedAMIModel430(\n",
" \"w\", address=\"GPIB::4::INSTR\", pyvisa_sim_file=\"AMI430.yaml\", terminator=\"\\n\"\n",
")\n",
"\n",
"print(mag_w.log.logger.name)\n",
"print(mag_w.switch_heater.log.logger.name)\n",
"print(mag_x.log.logger.name)"
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"The logger name is built from the driver class followed by the instrument's `name_parts`, so scoped loggers mirror the instrument hierarchy: a submodule logger is a child of its instrument's logger, which is a child of a per-driver-class logger, which is a child of the shared logger. Any level configured on `qcodes.instrument.instrument_base`, for instance via `logger_levels` in `qcodesrc.json`, is therefore still inherited.\n",
"\n",
"The scope is looked up on the root instrument when the instrument is created, so it has to be set before instantiating the driver and applies to the whole instrument.\n",
"\n",
"The level of a single instrument can now be changed without affecting any other instrument:"
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"mag_w.log.logger.setLevel(logging.DEBUG)\n",
"\n",
"print(logging.getLevelName(mag_w.log.logger.getEffectiveLevel()))\n",
"print(logging.getLevelName(mag_x.log.logger.getEffectiveLevel()))"
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"Because there is a node per driver class, you can also raise the level for *every* instrument of a driver at once, which is typically what you want while developing or debugging that driver. This works for instruments that do not exist yet, so it can equally well be put in the `logger_levels` section of `qcodesrc.json`:"
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"logging.getLogger(\"qcodes.instrument.instrument_base.ScopedAMIModel430\").setLevel(\n",
" logging.DEBUG\n",
")\n",
"\n",
"mag_v = ScopedAMIModel430(\n",
" \"v\", address=\"GPIB::2::INSTR\", pyvisa_sim_file=\"AMI430.yaml\", terminator=\"\\n\"\n",
")\n",
"\n",
"# both instruments of this driver are now at DEBUG, mag_x is untouched\n",
"for inst in (mag_w, mag_v):\n",
" print(inst.name, logging.getLevelName(inst.log.logger.getEffectiveLevel()))\n",
"print(mag_x.name, logging.getLevelName(mag_x.log.logger.getEffectiveLevel()))"
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"A few things are worth keeping in mind:\n",
"\n",
"* QCoDeS attaches its console and file handlers to the **root** logger and those handlers have their own levels. Lowering the level of an instrument logger to `DEBUG` only becomes visible if the handler level lets the record through, so combine it with `console_level` or `handler_level` as shown above.\n",
"* Setting `instrument.log.logger.propagate = False` only stops records from reaching *ancestor* handlers, including QCoDeS' root handlers — it is not a general mute. Handlers attached directly to that logger still run, and VISA records travel through the separate `instrument.visa_log`, which propagates independently. Submodule loggers are descendants, so their records are stopped at the instrument as well.\n",
"* Python's logging registry keeps every logger it creates for the lifetime of the process, and closing an instrument does not release its logger. Instruments with stable names reuse their existing logger, so a normal station is unaffected, but a workload that repeatedly creates instruments under *dynamically generated* names accumulates one registry entry per distinct name while this scope is enabled.\n",
"\n",
"Note also that parameters log through the logger of their *root* instrument, so messages from a channel parameter appear on the root instrument's logger rather than on the channel's own logger."
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"mag_w.close()\n",
"mag_v.close()"
]
},
{
"cell_type": "markdown",
"metadata": {},
Expand Down
72 changes: 70 additions & 2 deletions src/qcodes/instrument/instrument_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import collections.abc
import logging
import warnings
from typing import TYPE_CHECKING, Any, ClassVar, cast
from typing import TYPE_CHECKING, Any, ClassVar, Literal, cast

import numpy as np
from typing_extensions import TypedDict, TypeVar, deprecated
Expand Down Expand Up @@ -37,6 +37,21 @@
"TSubmodule", bound="InstrumentModule | ChannelTuple", default="InstrumentModule"
)

LoggerScope = Literal["shared", "instrument"]
"""
Naming scope used for the logger behind :attr:`InstrumentBase.log` (and
:attr:`~qcodes.instrument.VisaInstrument.visa_log`).

``"shared"``
All instruments share a single logger.
``"instrument"``
Each instrument gets its own logger, named after the class of its
``root_instrument`` followed by its ``name_parts``. This creates one logger
per driver class, one per instrument and one per submodule, each a child of
the previous one, so a level configured on any of them applies to
everything below it.
"""


class InstrumentBaseKWArgs(TypedDict):
"""
Expand Down Expand Up @@ -71,6 +86,30 @@ class InstrumentBase(MetadatableWithName, DelegateAttributes):

"""

default_logger_scope: ClassVar[LoggerScope] = "shared"
"""
Logger naming scope used for :attr:`log` (and
:attr:`~qcodes.instrument.VisaInstrument.visa_log`).

By default all instruments share a single logger. Set this to
``"instrument"`` in the definition of a driver class to give each of its
instruments a logger of its own, so that log levels and handlers can be
configured per instrument::

class MyDriver(VisaInstrument):
default_logger_scope = "instrument"

The resulting loggers are named
``qcodes.instrument.instrument_base.<DriverClass>.<name_parts>``, so a
level can be set for a whole driver class, a single instrument, or a single
submodule.

The scope is resolved once, when the :meth:`root_instrument` is created,
and every submodule uses the scope of its root. It therefore applies to the
whole instrument, including submodules added later, and changing it
afterwards has no effect on existing instruments.
"""

def __init__(
self,
name: str,
Expand Down Expand Up @@ -118,9 +157,38 @@ def __init__(
# This is needed for snapshot method to work
self._meta_attrs = ["name", "label"]

self.log: InstrumentLoggerAdapter = get_instrument_logger(self, __name__)
root = self.root_instrument
self._logger_scope: LoggerScope = (
type(self).default_logger_scope if root is self else root._logger_scope
)

self.log: InstrumentLoggerAdapter = get_instrument_logger(
self, self._logger_name(__name__)
)
self.log.debug("Created instrument: %s", self.full_name)

def _logger_name(self, base: str) -> str:
"""
Name of the logger to use, derived from ``base`` according to the
logger scope resolved when the :meth:`root_instrument` was created.

Under the ``"instrument"`` scope the name is built from the class of
the :meth:`root_instrument` followed by :meth:`name_parts`, e.g.
``<base>.MyDriver.myinst.ChanA``. That gives one node per driver class,
one per instrument and one per submodule, so a level can be configured
for a whole driver, a single instrument, or a single channel, and each
is inherited by everything below it.

Args:
base: Name of the shared logger that this instrument would use if
no scope was configured.

"""
if self._logger_scope == "instrument":
root = self.root_instrument
return ".".join((base, type(root).__name__, *self.name_parts))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would basically be equivalent to replacing base which is currently the name of this module qcodes.instrument.instrument_base with the name of the module that the instrument is defined in. qcodes.insrument.instrument_vendor.instrument_filename. I think I agreee that I would prefer this

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One blocker for replacing base with the name of the module the instrument is defined in: _logger_name is called with two different bases, name for self.log and VISA_LOGGER for self.visa_log. Drop base, and those two become the same logger name, so driver messages and wire traffic can no longer be separated at all. Module-only also wouldn't distinguish AMIModel430 from AMIModel4303D, since both live in AMI430_visa.py.

The problem is:

vendor_a.Model372  ->  qcodes.instrument.instrument_base.Model372   ┐ same
vendor_b.Model372  ->  qcodes.instrument.instrument_base.Model372   ┘ logger

Not sure if vendors will realistically have the same module number, but perhaps these other sources could cause a problem:

  • Same driver, two sources: a driver in qcodes.instrument_drivers.X and a fork/variant in qcodes_contrib_drivers or a local copy. Same class name by construction, since one is derived from the other.
  • Notebook drivers: a user's quick class MyDriver(VisaInstrument) in main, which collides with anyone else's MyDriver.
  • Subclassing in place: class AMIModel430(AMIModel430) style local tweaks.

In the category "same lineage, different module". Copilot's finding is technically correct but low severity: it needs a self-inflicted name clash, and the consequence is a shared log level, not data loss or a crash.

We could do the following:

     def _logger_name(self, base: str) -> str:
         root = self.root_instrument
         if root.default_logger_scope == "instrument":
-            return ".".join((base, type(root).__name__, *self.name_parts))
+            return ".".join((base, full_class(root), *self.name_parts))
         return base

So it would look like:

qcodes.instrument.instrument_base.qcodes.instrument_drivers.american_magnetics.AMI430_visa.AMIModel430.inst
qcodes.instrument.instrument_base.qcodes.instrument_drivers.tektronix.AWG5208.TektronixAWG5208.inst
                                  └──────────────── full_class ────────────────┘

Which is safer but very verbose. Another alternative is leaving it as is and documenting the behavior.

return base

@property
def label(self) -> str:
"""
Expand Down
2 changes: 1 addition & 1 deletion src/qcodes/instrument/ip_to_visa.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def __init__(
newkwargs = {kw: val for (kw, val) in kwargs.items() if kw not in ipkwargs}

Instrument.__init__(self, name, **newkwargs)
self.visa_log = get_instrument_logger(self, VISA_LOGGER)
self.visa_log = get_instrument_logger(self, self._logger_name(VISA_LOGGER))

##################################################
# __init__ of VisaInstrument
Expand Down
2 changes: 1 addition & 1 deletion src/qcodes/instrument/visa.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ def __init__(
timeout = self.default_timeout

super().__init__(name, **kwargs)
self.visa_log = get_instrument_logger(self, VISA_LOGGER)
self.visa_log = get_instrument_logger(self, self._logger_name(VISA_LOGGER))

self.timeout: Parameter[float | None, Self] = self.add_parameter(
"timeout",
Expand Down
Loading
Loading