chore: Improve local subpixel warnings and docstrings#3252
chore: Improve local subpixel warnings and docstrings#3252
Conversation
caseyflex
force-pushed
the
chore/casey/modewarning
branch
from
96ace94 to
932579b
Compare
caseyflex
force-pushed
the
chore/casey/modewarning
branch
from
932579b to
e86822e
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
caseyflex
force-pushed
the
chore/casey/modewarning
branch
from
e86822e to
f37b815
Compare
Diff CoverageDiff: origin/develop...HEAD, staged and unstaged changes
Summary
|
tylerflex
left a comment
tylerflex
left a comment
There was a problem hiding this comment.
a couple comments, also same thing about the use of quotes in the pip install extras, I'm not sure if it's just a Mac thing. It trips me up from time to time, but now that I know about it, I know how to fix it. just worried some users might get no matches found: tidy3d[extras] which is what I see on Mac if I copy that syntax.
| log_once=True, | ||
| ) | ||
| # only warn if tidy3d-extras local subpixel is not enabled | ||
| if not tidy3d_extras["use_local_subpixel"]: |
There was a problem hiding this comment.
won't this also warn If this is left to default of None? should it be if ... is False?
There was a problem hiding this comment.
I actually think I want this to warn if left at the default of None, if tidy3d-extras isn’t installed (because then subpixel won’t work out the box, and the warning then is the current desired behavior). The warning is updated to recommend extras as well though
There was a problem hiding this comment.
Note that this value im checking is whether extras package loaded, not the config flag
There was a problem hiding this comment.
Although if the config flag is false, this decorator will explicitly unload extras
There was a problem hiding this comment.
Basically this value tells you exactly whether subpixel is actually being used in the current function
There was a problem hiding this comment.
ok I see. yea I thought this was the config flag.
From my perspective as a user, it should just be a warning if for whatever reason I'm not using subpixel. If I am, it should be quiet? is that consistent with your thinking too and the logic here?
There was a problem hiding this comment.
I guess another question is whether the current logic now prints 2 warnings (one if tidy3d extras not installed and one if subpixel is not used?) that would be less ideal
There was a problem hiding this comment.
Yeah it should only warn here if subpixel isn’t being used.
There usually should not be two warnings. Only this one if subpixel is not being used. There will be another warning if tidy3d-extras is installed but failed to load for another reason like user permissions, but I think that’s good
| - Changed the interpretation of `waist_distance` and `waist_distances` for backward-propagating Gaussian beams (`GaussianBeam`, `AstigmaticGaussianBeam`, `GaussianBeamProfile`, `AstigmaticGaussianBeamProfile`). Previously, the waist position was interpreted relative to the directed propagation axis, meaning switching `direction` from `+` to `-` would also flip the waist position in the global reference frame. Now, the waist position is defined consistently for both directions: a positive `waist_distance` always places the beam waist behind the source/monitor plane (toward the negative normal axis), regardless of propagation direction. This ensures reciprocity between Gaussian sources and overlap monitors used in port-based S-matrix calculations. Users with existing simulations using backward-propagating Gaussian beams with non-zero waist distances may need to adjust their values. | ||
|
|
||
| ### Changed | ||
| - Improved `ModeSolver.solve()` to suppress the accuracy warning when local subpixel averaging is enabled via `tidy3d-extras`, and expanded docstrings for `ModeSolver.solve()`, `ModeSimulation.run_local()`, `Simulation.epsilon()`, and `Simulation.epsilon_on_grid()` to document the `config.simulation.use_local_subpixel` option. |
There was a problem hiding this comment.
might be better under "fixed"
2 participants
Fixes unclear warnings / docstrings reported by Tyler.
Related FAQ PR: flexcompute/tidy3d-faq#60
Note
Low Risk
Primarily documentation/logging changes with a small conditional change to when a warning is emitted; no solver algorithms or numerical outputs are modified.
Overview
Updates local-mode-solver behavior so
ModeSolver.solve()only emits its “use remote mode solver for better accuracy” warning whentidy3d-extraslocal subpixel averaging is not active, and adds/expands docstrings to consistently describe theconfig.simulation.use_local_subpixeltoggle acrossModeSolver.solve(),ModeSimulation.run_local(),Simulation.epsilon(), andSimulation.epsilon_on_grid().Improves user-facing documentation (
docs/configuration/reference.rst,docs/extras/index.rst) to spell out the tri-state semantics (True=require extras,False=staircasing,None=use when available), and adds tests ensuring the warning is suppressed/enabled appropriately.Written by Cursor Bugbot for commit f37b815. This will update automatically on new commits. Configure here.