minor fix bloch validator #1930
Conversation
tidy3d/components/simulation.py
Outdated
| # "order 0 will not correspond to the angle of propagation " | ||
| # "of the source. Consider using 'BlochBoundary.from_source()'.", | ||
| # custom_loc=["boundary_spec", "xyz"[dim]], | ||
| # ) |
There was a problem hiding this comment.
The way I read this, this was specifically added to avoid confusion when using such a source + a DiffractionMonitor. How about we only warn in that case then, i.e. check if a DiffractionMonitor is present rather than say "if using a diffraction monitor". This should fix your adjoint-specific annoyance with these warnings.
I remember at least one user was confused about this at the time.
There was a problem hiding this comment.
So I tried doing this, but the way our validators are set up in simulation, these don't have access to the monitors unfortunately. If I try to change anything, it breaks a ton of tests.. not sure if it's better to try to just rearrange things in simulation or simply remove the warning
There was a problem hiding this comment.
I came up with a solution that works, just added a separate validator for "monitors" and configured things to only check this condition if DiffractionMonitors are present there. seems to work properly.
tylerflex
force-pushed
the
tyler/fix/bloch
branch
from
bfaff3f
to
4ef9d3c
Compare
docs/notebooks
Outdated
There was a problem hiding this comment.
develop submodule is out of date..
|
This still doesn't look quite right though. The warning about the source k not being an integer multiple of the Bloch vector should always be there, even without diffraction monitors. This is because this setup seems unphysical (the source does not match the boundary conditions) and is extremely unlikely to be correct. I'm not sure why we left it as warning and not a straight out error but either way it should be there even without DiffractionMonitor. |
Unless I'm mistaken, this is how I have it set up. There are two validators, one checks that k is an integer multiple of the Bloch vector and is always triggered. The other checks that the Bloch vector is exactly equal to k, and is only checked now if a diffraction monitor is present. Could you take another look and verify whether it's consistent? |
tylerflex
force-pushed
the
tyler/fix/bloch
branch
from
4ef9d3c
to
5af4211
Compare
This was just something I noticed. The old code led to false positive warnings if the test_val was just a little bit less than an integer.. this is more robust.