Field projection validators #1288
Conversation
shashwat-sh
force-pushed
the
shash/warnings
branch
2 times, most recently
from
7188e6a
to
5bf2a09
Compare
| run_time=1e-12, | ||
| monitors=[monitor_n2f], | ||
| ) | ||
| assert_log_level(log_capture, "WARNING") |
There was a problem hiding this comment.
note, if you want to test that the next block of code is triggering a new warning, you should log_capture.clear() before entering that block. Otherwise it will still capture the first warning. As written, it is not testing that a new warning is logged with each block of code.
There was a problem hiding this comment.
Thanks, I've made the change
shashwat-sh
force-pushed
the
shash/warnings
branch
from
5bf2a09
to
03449e2
Compare
| """Warn if field projection observation points are behind surface projection monitors.""" | ||
| # This validator is in simulation.py rather than monitor.py because volume monitors are | ||
| # eventually converted to their bounding surface projection monitors, in which case we | ||
| # do not want this validator to be triggered. |
There was a problem hiding this comment.
Hmmm. Is there some pydantic magic than can allow us to choose whether to trigger this validator if it were in the monitors @tylerflex ? I don't know if we want to go this way but it may be a bit better in the long run, e.g. consider a standalone field projection solver like we've been thinking of implementing. Well, for that we could also move this validator to validators.py and import it both here and in other solvers.
There was a problem hiding this comment.
Not sure exactly what kind of magic we need to implement. But I'd say in general let's use less magic. Especially since if we do a pydantic v2 port we'll have to rethink it
There was a problem hiding this comment.
I guess I'm just asking if there's a way to dynamically disable some validator when initializing a model. But we could live without it.
There was a problem hiding this comment.
Besides just an if statement in the validator that returns if triggered? We could maybe make it a post init validation?
There was a problem hiding this comment.
Not sure I understand, what I'm asking is if I can do something like Model(**model_dict) and add some kwarg or set a config somehow that would skip that particular validator. This is for when we need to initialize something intrenally without triggering the warnign.
There was a problem hiding this comment.
I think anything you pass to the models init method will be present in the values dict in the validator. So you could add a flag in there. But then you'd have to pop it from values at some point
There was a problem hiding this comment.
One of these things that should work in theory but might not when trying it out but worth a shot
There was a problem hiding this comment.
If there's no pydantic native way then probably not worth it.
There was a problem hiding this comment.
I guess let's just merge this as is.
There was a problem hiding this comment.
Sounds good, I can do that now
|
|
||
| center = np.array(monitor.center) - np.array(monitor.local_origin) | ||
| pts = [np.array(i) for i in [x, y, z]] | ||
| normal_displacement = pts[normal_ind] - center[normal_ind] |
There was a problem hiding this comment.
Is doing it this way easier (and also is it equivalent?) to checking if theta < 0?
There was a problem hiding this comment.
The angle defined within the monitor is relative to the local origin, so we'd need to do some conversions anyway because theta < 0 in the local system may still not be "backwards" in the global system, relative to the monitor's position, depending on proj_distance. So I decided to always convert to the global system in Cartesian coords and treat all three cases in the same way - not sure if it's the best way
There was a problem hiding this comment.
another way could be to always convert to spherical coords in the global system and then check if the global theta < 0
far_field_approxisTrue