Improve the Verbosity class in python/verbosity_mgr.py - #3305
Merged
Conversation
stevengj
reviewed
Sep 10, 2026
stevengj
reviewed
Sep 10, 2026
stevengj
reviewed
Sep 10, 2026
move implementation details from docstring to comment
Co-authored-by: Steven G. Johnson <stevenj@mit.edu>
stevengj
approved these changes
Sep 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
python/verbosity_mgr.pydefines the singleton that fronts two C globals —meep::verbosity(src/mympi.cpp:147, default 1) andmpb_verbosity(libmpbsrc/util/verbosity.c, default 1) — exposed to Python by SWIG asmp.cvar.verbosityand_mpb.cvar.verbosity. It is the mechanism behindmp.verbosity(0)and the ~45verbosity.meep > 0/verbosity.mpb > 0guards insimulation.py,solver.py, andvisualization.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
make_property()doessetattr(Verbosity, name, property(...))— the base class, nottype(self).VerbosityForTestinpython/tests/test_verbosity_mgr.pytherefore graftsfoo/barpermanently onto the productionVerbosityclass, and oncereset()empties_cvarsthose properties raiseKeyError.verbosity.meep = 2) never updates_master_verbosity, soget(),int(),repr(), and every comparison silently go stale.__eq__is defined without__hash__, so Python sets__hash__ = None.__init__re-runs on everyVerbosity()call even though__new__returns the existing instance, so a bareVerbosity()registers a throwaway_dummyunder a manufactured namecvar_Nthat is then managed forever.add_verbosity_var()only callsset()for the first flag, somp.verbosity(0)followed byimport meep.mpbleavesverbosity.mpbat the C default of 1 instead of 0.copy.deepcopy(mp.Verbosity)corrupts the singleton.__new__returns the live instance, thencopyoverwrites its__dict__with copies of the cvars — after whichmp.verbosity(0)writes to dead objects and the C globals never change again.Rough edges
0..3range check is duplicated inset()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:28re-registersmp.cvaras"meep"(a no-op) and then readsverbosity.mpb, a name that exists only becausempb.ihappened to run first.TypeErroron non-numeric operands instead of returningNotImplemented.Additions
VerbosityLevelIntEnum,temporary(level)context manager,__index__,__delattr__,__dir__, one shared_check_level()validator (TypeErrorfor non-integral — which also makesnp.int64work, where it previously hit a confusing SWIG error),NotImplementedon non-numeric comparisons, and type hints.Also:
python/solver.pyandpython/mpb.inow use the already-publicadd_verbosity_var()instead of "constructing" a singleton that already exists;VerbosityLevelexported viameep.i, additional tests,@@ Verbosity.temporary @@added to the doc template.