Skip to content

Add QBXRefinementMode enum, deprecate _disable_refinement#310

Merged
inducer merged 3 commits intomainfrom
copilot/add-refinement-mode-enum
Apr 1, 2026
Merged

Add QBXRefinementMode enum, deprecate _disable_refinement#310
inducer merged 3 commits intomainfrom
copilot/add-refinement-mode-enum

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 19, 2026

_disable_refinement was 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

from pytential.qbx import QBXRefinementMode, QBXRefinementNeededError

# Default: refine as needed (unchanged behavior)
lpot = QBXLayerPotentialSource(..., refinement_mode=QBXRefinementMode.REFINE)

# Skip refinement silently (replaces _disable_refinement=True)
lpot = QBXLayerPotentialSource(..., refinement_mode=QBXRefinementMode.NO_REFINEMENT)

# Run checks, raise QBXRefinementNeededError if refinement would be required
lpot = QBXLayerPotentialSource(..., refinement_mode=QBXRefinementMode.COMPLAIN)

Changes

  • pytential/qbx/refinement.py: Added QBXRefinementMode enum (REFINE, NO_REFINEMENT, COMPLAIN) and QBXRefinementNeededError exception. Updated _refine_qbx_stage1 and _refine_qbx_stage2 to check refinement_mode; the COMPLAIN path 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: bool attribute with refinement_mode: QBXRefinementMode. Both __init__ and copy() accept the new refinement_mode parameter. _disable_refinement is retained as a deprecated parameter that maps to the equivalent mode with a DeprecationWarning. QBXRefinementMode and QBXRefinementNeededError are exported from pytential.qbx.

  • test/extra_int_eq_data.py: Updated to use refinement_mode instead of _disable_refinement.

Original prompt

This section details on the original issue you should resolve

<issue_title>Add a refinement mode: don't refine, but error if stage-1 or stage-2 refinement is needed</issue_title>
<issue_description>Do this by deprecating _disable_refinement and adding an enum for "refinement mode", with "no refinement", "complain", and "refine" as modes.</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 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.

Copilot AI changed the title [WIP] Add a refinement mode with no refinement and complain option Add QBXRefinementMode enum, deprecate _disable_refinement Mar 19, 2026
Copilot AI requested a review from inducer March 19, 2026 15:14
@inducer inducer force-pushed the copilot/add-refinement-mode-enum branch from 560ff14 to 8dce758 Compare April 1, 2026 15:10
@inducer inducer marked this pull request as ready for review April 1, 2026 15:17
Copilot AI review requested due to automatic review settings April 1, 2026 15:17
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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) plus QBXRefinementNeededError, and wired them into stage-1/stage-2 refinement.
  • Updated QBXLayerPotentialSource to accept/store refinement_mode and to map deprecated _disable_refinement to the new mode with a DeprecationWarning.
  • 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.

@inducer inducer enabled auto-merge (rebase) April 1, 2026 15:26
@inducer inducer merged commit c1be40d into main Apr 1, 2026
12 checks passed
@inducer inducer deleted the copilot/add-refinement-mode-enum branch April 1, 2026 15:46
Copy link
Copy Markdown
Collaborator

@alexfikl alexfikl left a comment

Choose a reason for hiding this comment

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

I missed this before, but left a couple of questions. 😁

What was the use case? To be sure that it doesn't actually refine?

Comment on lines +91 to +93
.. autoclass:: QBXRefinementMode

.. autoclass:: QBXRefinementNeededError
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.

Why are these documented both here and in qbx.refinement?


class _not_provided: # noqa: N801
pass
NOT_PROVIDED = Sentinel("NOT_PROVIDED")
Copy link
Copy Markdown
Collaborator

@alexfikl alexfikl Apr 1, 2026

Choose a reason for hiding this comment

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

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.

Comment on lines +132 to +134
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.
Copy link
Copy Markdown
Collaborator

@alexfikl alexfikl Apr 1, 2026

Choose a reason for hiding this comment

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

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
Copy link
Copy Markdown
Collaborator

@alexfikl alexfikl Apr 1, 2026

Choose a reason for hiding this comment

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

Not a big fan of COMPLAIN (complain about what? sounds like it just throws some warnings). Maybe something like ERROR_ON_REFINEMENT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a refinement mode: don't refine, but error if stage-1 or stage-2 refinement is needed

4 participants