Cylinder with sidewall angle #571
Conversation
tidy3d/components/geometry.py
Outdated
| @@ -1359,7 +1422,9 @@ def _surface_area(self, bounds: Bound) -> float: | |||
|
|
|||
|
|
|||
| class Cylinder(Centered, Circular, Planar): | |||
| """Cylindrical geometry. | |||
| """Cylindrical geometry with optional sidewall angle along axis | |||
| direction. When ``sidewall_angle`` is nonzero, it's in fact a | |||
There was a problem hiding this comment.
I'd recommend replacing "it's in fact a" with "the shape is a"
tidy3d/components/geometry.py
Outdated
| 0.0, | ||
| title="Sidewall angle", | ||
| description="Angle of the sidewall. " | ||
| "``sidewall_angle=0`` (default) specifies vertical wall, " |
There was a problem hiding this comment.
A few grammar edits
``sidewall_angle=0`` (default) specifies a vertical wall,
``0<sidewall_angle<np.pi/2`` specifies a base that is larger than the top,
and ``-np.pi/2<sidewall_angle<0`` specifies a base that is smaller than the top.
Also, I wonder if we should add another line saying that the "base" is defined by the reference_plane field. ie
The base location is defined by the ``reference_plane`` field.
There was a problem hiding this comment.
How about just changing base to bottom?
There was a problem hiding this comment.
I feel like maybe "bottom" and "top" are maybe a bit confusing to use if they will be the strings used in the reference_plane. For example, if we say here that "bottom is larger than top" and set reference_plane="top", it might be ambiguous.
There was a problem hiding this comment.
one idea is to change reference_plane to be called base_position (or something similar) and make it a float between -1 and 1, with -1 meaning what is "bottom" now (default) and 1 meaning "top". Any ideas?
There was a problem hiding this comment.
That's a good point. But I feel that adding another variable complicates the problem. Currently we define "top" and "bottom" in the docstring of reference_plane: "E.g. if axis=1, bottom refers to the negative side of the y-axis". Even with an additional float base_position, we still need to iterate this in both the docstrings of base_position and reference_plane . How about this: we don't mention about "top" etc. at all in the docstring of sidewall_angle, rather, we can say,
sidewall_angle=0(default) specifies a vertical wall;0<sidewall_angle<np.pi/2specifies a shrinking cross section along theaxis-direction; and-np.pi/2<sidewall_angle<0specifies an expanding cross section along theaxis-direction.
And we keep the docstring of reference_plane as it is.
There was a problem hiding this comment.
I was actually suggesting we replace reference_plane with base_position for example, the base_position specifies the polygon base between -1 and 1 with the two extremes meaning "bottom" (min value of slab along axis) and "top" (max value along axis). So it would replace reference_plane. I think what you've done to change the docstring is probably fine though. Feel free to merge this.
weiliangjin2021
force-pushed
the
weiliang/slanted_cylinder
branch
from
0a62015
to
8b83b05
Compare
sidewall_angleetc. has been shifted toPlanar.