-
Notifications
You must be signed in to change notification settings - Fork 124
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
Included PolarGrid class. right now it assumes no ghost cells #136
base: main
Are you sure you want to change the base?
Conversation
pyro/mesh/patch.py
Outdated
t1 = y[+1:, +1:] | ||
|
||
return (r1 ** 3 - r0 ** 3) * (np.cos(t1) - np.cos(t0)) * (-2.0 * np.pi) / 3.0 | ||
# return r1 |
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.
r**3 makes this a volume, not a face area, right? shouldn't this just be
dr * r (t1 - t0) ?
for the area of a wedge in the r-theta polar plane?
:)
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.
sorry, that would be the volume
for the area, you should have an area_x and area_y function
and those should give just r*theta and dr respectively.
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.
:)
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.
This function computes dr and dtheta. :)
pyro/mesh/patch.py
Outdated
# return r1 | ||
|
||
def cell_vertices(self): | ||
""" |
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.
isn't this already what self.xl and self.xr provide?
:)
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 did not really understand what they are, and decided to write my own function. ;)
pyro/mesh/patch.py
Outdated
# pylint: disable=too-many-instance-attributes | ||
|
||
def __init__(self, nx, ny, ng=1, | ||
xmin=0.0, xmax=1.0, ymin=0.0, ymax=1.0): |
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 don't think you need init, since it looks the same as the base class
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.
:)
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 left if for later if it will be needed to be changed.
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.
It might be better to call super().__init__(nx, ny, ...)
in that case
Co-authored-by: Eric T. Johnson <[email protected]>
Co-authored-by: Eric T. Johnson <[email protected]>
you should only need to add 3 functions to Grid2d: area_x() it might also be good to add an assertion on the angle (y) the ghost cells should be handled just fine by what is already in the class |
pyro/mesh/patch.py
Outdated
The shape of the returned array is (ni, nj). | ||
""" | ||
tr = lambda arr: arr.transpose(1, 2, 0) | ||
x = self.cell_vertices()[:,0] |
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 you use the data in the class instead of recomputing the vertices yourself?
…into polar-coords
…into polar-coords
…into polar-coords
|
||
# pylint: disable=too-many-instance-attributes | ||
|
||
def area_x(self): |
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.
this is the area through a y (theta) face. area_x()
should return the area through the x faces, which is r_{i-1/2} dtheta
return area | ||
|
||
def area_y(self): | ||
""" |
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.
this should be the area through a y face, which is just dr
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.
but y face is dtheta
No description provided.