Skip to content

Improve the Verbosity class in python/verbosity_mgr.py - #3305

Merged
oskooi merged 5 commits into
NanoComp:masterfrom
oskooi:verbosity_improvements
Sep 11, 2026
Merged

Improve the Verbosity class in python/verbosity_mgr.py#3305
oskooi merged 5 commits into
NanoComp:masterfrom
oskooi:verbosity_improvements

Conversation

@oskooi

@oskooi oskooi commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Context

python/verbosity_mgr.py defines the singleton that fronts two C globals — meep::verbosity (src/mympi.cpp:147, default 1) and mpb_verbosity (libmpb src/util/verbosity.c, default 1) — exposed to Python by SWIG as mp.cvar.verbosity and _mpb.cvar.verbosity. It is the mechanism behind mp.verbosity(0) and the ~45 verbosity.meep > 0 / verbosity.mpb > 0 guards in simulation.py, solver.py, and visualization.py.

The class works for its dominant use, but reading it turns up five genuine defects plus several rough edges. None are caused by the API's shape, so the fix is targeted repair rather than a redesign: the public surface (verbosity(N), verbosity.meep, verbosity.mpb, get/set/get_all/reset, the comparison operators) stays as it is and no read site changes.

Defects

  1. Subclass property leak. make_property() does setattr(Verbosity, name, property(...)) — the base class, not type(self). VerbosityForTest in python/tests/test_verbosity_mgr.py therefore grafts foo/bar permanently onto the production Verbosity class, and once reset() empties _cvars those properties raise KeyError.
  2. Stale global level. Per-flag assignment (verbosity.meep = 2) never updates _master_verbosity, so get(), int(), repr(), and every comparison silently go stale.
  3. Unhashable singleton. __eq__ is defined without __hash__, so Python sets __hash__ = None.
  4. Phantom flags. __init__ re-runs on every Verbosity() call even though __new__ returns the existing instance, so a bare Verbosity() registers a throwaway _dummy under a manufactured name cvar_N that is then managed forever.
  5. Late registration clobber an explicit setting. add_verbosity_var() only calls set() for the first flag, so mp.verbosity(0) followed by import meep.mpb leaves verbosity.mpb at the C default of 1 instead of 0.
  6. copy.deepcopy(mp.Verbosity) corrupts the singleton. __new__ returns the live instance, then copy overwrites its __dict__ with copies of the cvars — after which mp.verbosity(0) writes to dead objects and the C globals never change again.

Rough edges

  • The 0..3 range check is duplicated in set() and in the property setter; a non-integral level reaches the SWIG setter produces a confusing error.
  • reset() calls _init() on the instance it is about to drop, mutating state that outstanding references still use.
  • solver.py:28 re-registers mp.cvar as "meep" (a no-op) and then reads verbosity.mpb, a name that exists only because mpb.i happened to run first.
  • Comparison dunders raise TypeError on non-numeric operands instead of returning NotImplemented.

Additions

VerbosityLevel IntEnum, temporary(level) context manager, __index__, __delattr__, __dir__, one shared _check_level() validator (TypeError for non-integral — which also makes np.int64 work, where it previously hit a confusing SWIG error), NotImplemented on non-numeric comparisons, and type hints.

Also: python/solver.py and python/mpb.i now use the already-public add_verbosity_var() instead of "constructing" a singleton that already exists; VerbosityLevel exported via meep.i, additional tests, @@ Verbosity.temporary @@ added to the doc template.

Comment thread doc/docs/Python_User_Interface.md Outdated
Comment thread doc/docs/Python_User_Interface.md Outdated
Comment thread doc/docs/Python_User_Interface.md Outdated
move implementation details from docstring to comment
Co-authored-by: Steven G. Johnson <stevenj@mit.edu>
@oskooi
oskooi merged commit 6ef8896 into NanoComp:master Sep 11, 2026
6 checks passed
@oskooi
oskooi deleted the verbosity_improvements branch September 11, 2026 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants