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

Add geometry for 2d pillow connectivity to map the sphere and new pillow3d to map a spherical shell #321

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

pkestene
Copy link
Contributor

@pkestene pkestene commented Oct 16, 2024

  • add missing geometry for pillow connectivity for mapping a spherical shell
  • add new 2d geometry pillow_disk for mapping a 2d disk from unitsquare connectivity
  • add new 3d connectivity pillow3d and associated geometry to map a spherical shell

The transformation used in pillow to map the sphere are based upon:
"Logically rectangular finite volume methods with adaptive refinement on the sphere", Berger et al.
https://doi.org/10.1098/rsta.2009.0168

The transformation used in pillow_disk to map the disk are based upon:
"Logically rectangular grids and finite volume methods for PDEs in circular and spherical domains", Calhoun et al, SIAM Review, volume 50, Issue 4, January 2008.
https://doi.org/10.1137/060664094

This PR is a also a fix of #318 (dropped because pillow3d connectivity was erroneous).

@pkestene pkestene force-pushed the feature-pillow-sphere-2 branch 4 times, most recently from 4b3a07f to 6168dd5 Compare October 18, 2024 21:04
@hannesbrandt
Copy link
Collaborator

I've checked out the code and the resulting paraview output. Everything looks good to me. It will be nice to have these geometries available in p4est!

R = atof (argv[3]);
if (argc >= 5) {
iconfig = atoi (argv[4]);
if (iconfig >=0 && iconfig <=3) {
Copy link
Owner

Choose a reason for hiding this comment

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

Would it be safer to check the range using the names FIG3A to FIG3D?

@cburstedde
Copy link
Owner

cburstedde commented Oct 29, 2024

Cool, thanks! I'm thinking about unifying the naming. What about in 3D: just call it pillow instead of pillow3d. For symmetry with 2D, we may want to add in 3D a 1-tree pillow_sphere geometry if you like the idea.

Details: running p4estindent would be great (ignoring its changes wrt. whitespace in argument lists (use: type *variable)).

@pkestene
Copy link
Contributor Author

pkestene commented Nov 4, 2024

Cool, thanks! I'm thinking about unifying the naming. What about in 3D: just call it pillow instead of pillow3d. For symmetry with 2D, we may want to add in 3D a 1-tree pillow_sphere geometry if you like the idea.

Details: running p4estindent would be great (ignoring its changes wrt. whitespace in argument lists (use: type *variable)).

Ok, I'll modify the code to take into account your remarks, and come back shortly.

The geometry is named pillow_disk. There are actually four variants of the
geometry which correspond to the four grid mappings shown in figure 3 from
the following publication:

"Logically rectangular grids and finite volume methods for PDEs in circular and
spherical domains", Calhoun et al, SIAM Review, volume 50, Issue 4, January
2008.

See https://doi.org/10.1137/060664094
This is a geometry to be used with unitcube connectivity for mapping the solid
sphere. It is the 3d analog of the 2d pillow_disk geometry.
@pkestene pkestene force-pushed the feature-pillow-sphere-2 branch from d924914 to bb3e913 Compare December 20, 2024 13:43
@pkestene
Copy link
Contributor Author

Hi @cburstedde
I've updated this branch to address your comments and rebased on latest develop.

@pkestene
Copy link
Contributor Author

pkestene commented Jan 8, 2025

Hi @cburstedde

would it be possible to re-run ci, since the failure are related to the transient problem of test_loadsave2 (issue #304) but apparently not with the proposed modifications ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants