Add QBXRefinementMode enum, deprecate _disable_refinement#310
Conversation
Co-authored-by: inducer <352067+inducer@users.noreply.github.com>
560ff14 to
8dce758
Compare
There was a problem hiding this comment.
Pull request overview
This PR replaces the boolean _disable_refinement knob on QBXLayerPotentialSource with a more expressive QBXRefinementMode enum that supports “refine”, “skip refinement”, and “complain if refinement would be needed”, while keeping _disable_refinement as a deprecated compatibility parameter.
Changes:
- Added
QBXRefinementMode(REFINE,NO_REFINEMENT,COMPLAIN) plusQBXRefinementNeededError, and wired them into stage-1/stage-2 refinement. - Updated
QBXLayerPotentialSourceto accept/storerefinement_modeand to map deprecated_disable_refinementto the new mode with aDeprecationWarning. - Updated tests/config to use the new API and typing support (
typing_extensions.Sentinel, basedpyright settings/baseline).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
pytential/qbx/refinement.py |
Introduces refinement-mode enum/exception and adds COMPLAIN behavior in stage-1 and stage-2 refinement checks. |
pytential/qbx/__init__.py |
Replaces _disable_refinement with refinement_mode, adds deprecation mapping/warnings, and exports the new enum/exception. |
test/extra_int_eq_data.py |
Switches test configuration from _disable_refinement to refinement_mode. |
pyproject.toml |
Adds a declared dependency on typing-extensions (for Sentinel) and enables basedpyright experimental features. |
.basedpyright/baseline.json |
Updates the basedpyright baseline to reflect the new typing configuration/code. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
alexfikl
left a comment
There was a problem hiding this comment.
I missed this before, but left a couple of questions. 😁
What was the use case? To be sure that it doesn't actually refine?
| .. autoclass:: QBXRefinementMode | ||
|
|
||
| .. autoclass:: QBXRefinementNeededError |
There was a problem hiding this comment.
Why are these documented both here and in qbx.refinement?
|
|
||
| class _not_provided: # noqa: N801 | ||
| pass | ||
| NOT_PROVIDED = Sentinel("NOT_PROVIDED") |
There was a problem hiding this comment.
I'm getting this error on Sphinx 9.1.0:
<unknown>:1: WARNING: py:class reference target not found: NOT_PROVIDED [ref.class]
Not sure why Sphinx v8 works.. it should also not be able to find it? For what it's worth, _not_provided was ignored in conf.py.
| Do not perform any refinement, but raise a | ||
| :class:`QBXRefinementNeededError` if stage-1 or stage-2 refinement | ||
| would be required to satisfy the QBX refinement criteria. |
There was a problem hiding this comment.
Like copilot, I would also appreciate some mention that this ignores force_stage2_uniform_refinement_rounds. Maybe in the docs of refine_geometry_collection? 😁
| Executing global QBX without refinement is unlikely to give | ||
| accurate results. | ||
|
|
||
| .. attribute:: COMPLAIN |
There was a problem hiding this comment.
Not a big fan of COMPLAIN (complain about what? sounds like it just throws some warnings). Maybe something like ERROR_ON_REFINEMENT?
_disable_refinementwas a blunt boolean with no way to detect when refinement would be needed without actually doing it. Replaces it with a proper enum supporting three modes.New API
Changes
pytential/qbx/refinement.py: AddedQBXRefinementModeenum (REFINE,NO_REFINEMENT,COMPLAIN) andQBXRefinementNeededErrorexception. Updated_refine_qbx_stage1and_refine_qbx_stage2to checkrefinement_mode; theCOMPLAINpath runs all criteria checks and raises with a message identifying the violated criteria if any refinement would be triggered.pytential/qbx/__init__.py: Replaced_disable_refinement: boolattribute withrefinement_mode: QBXRefinementMode. Both__init__andcopy()accept the newrefinement_modeparameter._disable_refinementis retained as a deprecated parameter that maps to the equivalent mode with aDeprecationWarning.QBXRefinementModeandQBXRefinementNeededErrorare exported frompytential.qbx.test/extra_int_eq_data.py: Updated to userefinement_modeinstead of_disable_refinement.Original prompt
💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.