-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support uniform dimensions in TopologyDesignRegion
#1987
Conversation
@@ -166,12 +173,10 @@ def _check_params(params: anp.ndarray = None): | |||
@property | |||
def params_shape(self) -> typing.Tuple[int, int, int]: | |||
"""Shape of the parameters array in (x, y, z), given the ``pixel_size`` and bounds.""" | |||
# rmin, rmax = np.array(self.geometry.bounds) | |||
# lengths = rmax - rmin | |||
side_lengths = np.array(self.size) | |||
num_pixels = np.ceil(side_lengths / self.pixel_size) | |||
# TODO: if the structure is infinite but the simulation is finite, need reduced bounds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this TODO be removed?
More generally, in principle since we have the explicit uniform
argument now, it probably doesn't make that much sense to set to single pixel if the design region is infinite. We should just span the entire simulation instead. Thoughts @yaugenst-flex @tylerflex ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think the problem with that was that the invdes region has no information about the simulation, i.e. it can't determine the array shape from inf bounds because it can't intersect with the simulation domain.
would require more changes to make that work somehow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then maybe this should be a method of InverseDesign not a property of TopologyDesignRegion, or it should be a method that takes in a Simulation?
Not necessary to fix here but something to think about. It's just that the inf
behavior would be kind of unexpected to me especially compared to other Tidy3D components which we often send to inf when we just want to span the whole domain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More generally though it may be hard to come up with a simulation setup in which you want your topology region to span the whole domain.. Certainly seems fishy with PML, maybe in some cases with periodic boundaries?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah agree I don't think this would ever be correct with PMLs. periodic could make sense yeah
No description provided.