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

Update descriptions of dcEdge and dvEdge #99

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented May 13, 2024

The existing descriptions were not clear to developers.

@xylar
Copy link
Collaborator Author

xylar commented May 13, 2024

@mark-petersen, @proteanplanet, @matthewhoffman, as representatives form each affected component, could you let me know if you're okay with these changes to the descriptions of dcEdge and dvEdge?

<var name="dvEdge" type="real" dimensions="nEdges" units="m"
    description="The great circle distance between vertices bordering a given edge.  It is the length of the edge on the primal mesh."
/>
<var name="dcEdge" type="real" dimensions="nEdges" units="m"
    description="The great circle distance between cell centers bordering a given edge.  It is the length of the edge on the dual mesh."
/>

@xylar xylar force-pushed the mpas/update-edge-length-descriptions branch from 0c20d61 to a4ea7f9 Compare May 13, 2024 15:55
@matthewhoffman
Copy link
Collaborator

@xylar , thanks for tackling this. If we are adjusting these definitions, I would prefer some additional elaboration:

dvEdge:
The distance (great circle distance for spherical meshes, Euclidean distance for planar meshes) between the two vertices (defined by verticesOnEdge) located at the endpoints of a given edge. It is the length of the edge on the primal mesh.

dcEdge
The distance (great circle distance for spherical meshes, Euclidean distance for planar meshes) between the centers of the two cells (defined by cellsOnEdge) neighboring a given edge. It is the length of the edge on the primal mesh.

@xylar
Copy link
Collaborator Author

xylar commented May 13, 2024

The second one should be "dual", rather than "primal", right?

@xylar
Copy link
Collaborator Author

xylar commented May 13, 2024

Good point about planar vs. spherical.

@matthewhoffman
Copy link
Collaborator

The second one should be "dual", rather than "primal", right?

Yes, oops, good catch. Copy-and-paste error. :)

@xylar
Copy link
Collaborator Author

xylar commented May 14, 2024

@matthewhoffman, could you take a careful look at the update I just made? I want to make sure it's what you wanted.

@xylar
Copy link
Collaborator Author

xylar commented May 14, 2024

Taking @mark-petersen off as he's away.

Copy link
Collaborator

@matthewhoffman matthewhoffman left a comment

Choose a reason for hiding this comment

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

@xylar , I read it carefully out loud twice, and it looks good.

Copy link
Collaborator

@proteanplanet proteanplanet left a comment

Choose a reason for hiding this comment

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

Looks great. Thank you.

@xylar xylar removed the request for review from mark-petersen May 16, 2024 05:02
@xylar
Copy link
Collaborator Author

xylar commented May 16, 2024

@xylar
Copy link
Collaborator Author

xylar commented May 17, 2024

Never mind, there's nothing to update with generate_e3sm_namelist_files.py. Moving to E3SM...

xylar added 2 commits May 17, 2024 00:00
The existing descriptions were not clear to developers.
@xylar xylar force-pushed the mpas/update-edge-length-descriptions branch from 615eb91 to 802bac7 Compare May 17, 2024 05:00
@xylar
Copy link
Collaborator Author

xylar commented May 17, 2024

Closing in favor of E3SM-Project#6427

@xylar xylar closed this May 17, 2024
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.

3 participants