Skip to content

Error if projection monitor touches periodic/bloch boundaries in 3D simulations#1958

Open
QimingFlex wants to merge 2 commits intodevelopfrom
qiming/periodic_projection
Open

Error if projection monitor touches periodic/bloch boundaries in 3D simulations#1958
QimingFlex wants to merge 2 commits intodevelopfrom
qiming/periodic_projection

Conversation

@QimingFlex
Copy link
Contributor

@QimingFlex QimingFlex commented Sep 13, 2024

This validator raises an error in 3D simulations when a projection monitor touches periodic/Bloch boundaries. For 2D simulations, we allow periodic boundaries, which won't trigger this error. For 2D with Bloch boundaries, we already error it out in another PR where we just don't allow Bloch boundaries along the 0-dimension.

@QimingFlex QimingFlex force-pushed the qiming/periodic_projection branch 4 times, most recently from 01eff25 to f465496 Compare September 15, 2024 03:46
@QimingFlex QimingFlex marked this pull request as ready for review September 15, 2024 06:37
Copy link
Collaborator

@momchil-flex momchil-flex left a comment

Choose a reason for hiding this comment

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

Now that I think about this, I think field projection monitors will always be wrong if there are periodic or Bloch boundaries, regardless of whether they touch the boundaries or not. Either way, to do a correct projection, a periodic Green's function must be used. So I think the error should just be whenever there's any field projection monitor and any periodic/bloch boundaries together - for a 3D simulation, I cannot come up with a case in which this will work well even if e.g. the monitor is along xy and the boundaries are along z, it's again weird. And the error could suggest to the user to look into DiffractionMonitor, which is probably what they want to use in most cases, unless they have periodicity on one axis and not the other - that's something we don't really support now.

return val

@pydantic.validator("monitors", always=True)
@skip_if_fields_missing(["size", "boundary_spec"])
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess also "center"?

@QimingFlex
Copy link
Contributor Author

Now that I think about this, I think field projection monitors will always be wrong if there are periodic or Bloch boundaries, regardless of whether they touch the boundaries or not. Either way, to do a correct projection, a periodic Green's function must be used. So I think the error should just be whenever there's any field projection monitor and any periodic/bloch boundaries together - for a 3D simulation, I cannot come up with a case in which this will work well even if e.g. the monitor is along xy and the boundaries are along z, it's again weird. And the error could suggest to the user to look into DiffractionMonitor, which is probably what they want to use in most cases, unless they have periodicity on one axis and not the other - that's something we don't really support now.

Ah, I see. This makes sense. Previously, I thought projection with just one period was physically meaningful, but yeah it's not practically meaningful.

@QimingFlex QimingFlex force-pushed the qiming/periodic_projection branch from f465496 to f0c112a Compare October 4, 2024 21:55
@QimingFlex
Copy link
Contributor Author

@momchil-flex I updated the code to throw an error when a FieldProjectionMonitor is used with periodic/bloch boundaries. But it appears this sim object is used in many test functions and will throw a bunch of errors (some related to DiffractionMonitor) if I change the boundaries from periodic/bloch to PMLs. Could you give me some suggestions for the fix?

Copy link
Collaborator

@momchil-flex momchil-flex left a comment

Choose a reason for hiding this comment

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

Hm, yeah, I see the issue - not sure, will open it up for discussion.

if isinstance(monitor, AbstractFieldProjectionMonitor) and is_3d_simulation:
raise SetupError(
f"Using 'FieldProjectionMonitor' '{monitor.name}' "
f"with periodic/Bloch boundaries along the {axis} axis would lead to incorrect results. "
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think no need to print the axis as using the projection monitor with any periodic/Bloch boundaries is wrong.

Using 'FieldProjectionMonitor' '{monitor.name}' with periodic/Bloch boundaries will lead to incorrect results.

@momchil-flex momchil-flex added the 2.8 will go into version 2.8.* label Nov 5, 2024
@momchil-flex momchil-flex removed the 2.8 will go into version 2.8.* label Feb 27, 2025
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.

2 participants