Skip to content
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

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

ssagynbayeva
Copy link

No description provided.

t1 = y[+1:, +1:]

return (r1 ** 3 - r0 ** 3) * (np.cos(t1) - np.cos(t0)) * (-2.0 * np.pi) / 3.0
# return r1
Copy link
Collaborator

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?

:)

Copy link
Collaborator

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

:)

Copy link
Author

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. :)

# return r1

def cell_vertices(self):
"""
Copy link
Collaborator

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?
:)

Copy link
Author

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. ;)

# 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):
Copy link
Collaborator

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

:)

Copy link
Author

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.

Copy link
Collaborator

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

pyro/mesh/patch.py Outdated Show resolved Hide resolved
pyro/mesh/patch.py Outdated Show resolved Hide resolved
ssagynbayeva and others added 2 commits March 9, 2023 14:34
Co-authored-by: Eric T. Johnson <yut23@users.noreply.github.com>
Co-authored-by: Eric T. Johnson <yut23@users.noreply.github.com>
@zingale
Copy link
Collaborator

zingale commented Mar 9, 2023

I think that this is the geometry you want:

polar

@zingale
Copy link
Collaborator

zingale commented Mar 9, 2023

you should only need to add 3 functions to Grid2d:

area_x()
area_y()
volume()

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

The shape of the returned array is (ni, nj).
"""
tr = lambda arr: arr.transpose(1, 2, 0)
x = self.cell_vertices()[:,0]
Copy link
Collaborator

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?


# pylint: disable=too-many-instance-attributes

def area_x(self):
Copy link
Collaborator

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):
"""
Copy link
Collaborator

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

Copy link
Author

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

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.

None yet

3 participants