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

Ksagiyam/periodic extrusion #2582

Merged
merged 8 commits into from
Mar 8, 2023
Merged

Ksagiyam/periodic extrusion #2582

merged 8 commits into from
Mar 8, 2023

Conversation

ksagiyam
Copy link
Contributor

@ksagiyam ksagiyam commented Oct 13, 2022

Closes #2570

Enable periodic extrusion.

  • Need to do more testing, Done.
  • Need to fix I/O for this new mesh. Done.

Depends on: OP2/PyOP2#675 (Merged)

@ksagiyam ksagiyam force-pushed the ksagiyam/periodic_extrusion branch 7 times, most recently from e3be13d to 24663c0 Compare October 15, 2022 21:23
@ksagiyam ksagiyam marked this pull request as ready for review October 15, 2022 21:26
@ksagiyam ksagiyam force-pushed the ksagiyam/periodic_extrusion branch 7 times, most recently from f132f82 to 41108b1 Compare October 23, 2022 21:12
@@ -42,7 +42,7 @@ jobs:
- name: Build Firedrake
run: |
cd ..
./firedrake/scripts/firedrake-install $COMPLEX --venv-name build --tinyasm --disable-ssh --minimal-petsc --slepc --documentation-dependencies --install thetis --install gusto --install icepack --install irksome --install femlium --no-package-manager|| (cat firedrake-install.log && /bin/false)
./firedrake/scripts/firedrake-install $COMPLEX --venv-name build --tinyasm --disable-ssh --minimal-petsc --slepc --documentation-dependencies --install thetis --install gusto --install icepack --install irksome --install femlium --no-package-manager --package-branch firedrake ksagiyam/periodic_extrusion --package-branch pyop2 ksagiyam/periodic_extrusion|| (cat firedrake-install.log && /bin/false)
Copy link
Member

Choose a reason for hiding this comment

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

Revert before merge



@pytest.mark.parallel(nprocs=3)
def test_extruded_periodic_annulus():
Copy link
Member

Choose a reason for hiding this comment

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

Could do the same for a torus mesh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a comment in ExtrudedMesh doc.

@@ -2046,7 +2055,7 @@ def Mesh(meshfile, **kwargs):


@PETSc.Log.EventDecorator("CreateExtMesh")
def ExtrudedMesh(mesh, layers, layer_height=None, extrusion_type='uniform', kernel=None, gdim=None, name=None):
def ExtrudedMesh(mesh, layers, layer_height=None, extrusion_type='uniform', periodic=False, kernel=None, gdim=None, name=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be useful to have TorusMesh and AnnulusMesh functions in utility_meshes.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added AnnulusMesh and SolidTorusMesh.

@ksagiyam ksagiyam force-pushed the ksagiyam/periodic_extrusion branch 7 times, most recently from 8af256a to d07959e Compare February 22, 2023 13:44
Copy link
Member

@dham dham left a comment

Choose a reason for hiding this comment

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

Just fix docstrings.

distribution_name=None,
permutation_name=None,
):
"""Generate a solid toroidal mesh (with axis z) periodically extruding a disk mesh
Copy link
Member

Choose a reason for hiding this comment

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

Wrong docstring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

r,
nr=4,
nt=32,
distribution_parameters=None,
Copy link
Member

Choose a reason for hiding this comment

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

Not in docstring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.


:arg R: The major radius
:arg r: The minor radius
:kwarg nR: (optional), number of cells in the major direction (min 3, defaults to 8)
Copy link
Member

Choose a reason for hiding this comment

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

Don't put default in docstring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@ksagiyam ksagiyam force-pushed the ksagiyam/periodic_extrusion branch 3 times, most recently from b4c971c to 5fa9e65 Compare February 23, 2023 13:14
@ksagiyam ksagiyam force-pushed the ksagiyam/periodic_extrusion branch from 5fa9e65 to a8c1c7a Compare March 7, 2023 16:38
COMM_WORLD).
:kwarg radius: (optional) radius of the circle to approximate.
:kwarg degree: polynomial degree of coordinate space (e.g.,
cells are straight line segments if degree=1).
Copy link
Member

Choose a reason for hiding this comment

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

edges

@dham dham merged commit 4779660 into master Mar 8, 2023
@dham dham deleted the ksagiyam/periodic_extrusion branch March 8, 2023 16:49
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.

4 participants