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 source mesh modeling #5

Merged
merged 8 commits into from
Jul 22, 2023
Merged

Add source mesh modeling #5

merged 8 commits into from
Jul 22, 2023

Conversation

connoramoreno
Copy link
Collaborator

Adds ability to model a source mesh based on plasma equilibrium data. The mesh is structured in confinement space and transformed to a tetrahedral mesh in real space.

Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

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

Thanks @connoramoreno - hopefully this gets more compact and readable with these changes

source_mesh.py Outdated Show resolved Hide resolved
source_mesh.py Outdated Show resolved Hide resolved
source_mesh.py Show resolved Hide resolved
source_mesh.py Outdated Show resolved Hide resolved
source_mesh.py Outdated Show resolved Hide resolved
source_mesh.py Outdated Show resolved Hide resolved
source_mesh.py Outdated Show resolved Hide resolved
source_mesh.py Outdated Show resolved Hide resolved
source_mesh.py Outdated Show resolved Hide resolved
Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

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

Thanks for exploring the ravel_index version. I can imagine that you find it even less readable :) and I might not disagree. I think we need to talk it over in a s/w meeting. There are some standard patterns for this kind of thing that I think I can share better in person.

source_mesh.py Outdated Show resolved Hide resolved
source_mesh.py Outdated Show resolved Hide resolved
source_mesh.py Outdated Show resolved Hide resolved
source_mesh.py Outdated Show resolved Hide resolved
Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

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

A couple of little comments, but I'm going to merge anyway. We can create issues to consider how we might address this going forward.

# Initialize vertex index
i_vert = 0
# Compute vertices in Cartesian space
for i_phi in range(num_phi):
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 the vertices at i_phi = num_phi - 1 overlap with those at i_phi = 0?

I'm not really worried about the memory cost - it's not substantial - but thinking about robustness. The last set of triangles in phi are not connected to the first set. Since we only use this mesh for source sampling, that may not be an issue.

It also may result in complex code to avoid it by reusing the vertices from i_phi = 0

# Compute closed flux surface index for vertex
s = i_s * delta_s

for i_theta in range(num_theta):
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 the vertices at i_theta = num_phi - 1 overlap with those at i_theta = 0?

@gonuke gonuke merged commit 40ae617 into svalinn:main Jul 22, 2023
@connoramoreno connoramoreno deleted the source_mesh branch April 26, 2024 20:19
connoramoreno pushed a commit that referenced this pull request Sep 10, 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.

2 participants