Skip to content

Adding velocity option to isosurfaces refinement strategy - #7257

Open
hyunseong96 wants to merge 1 commit into
geodynamics:mainfrom
hyunseong96:isosurf_v
Open

Adding velocity option to isosurfaces refinement strategy#7257
hyunseong96 wants to merge 1 commit into
geodynamics:mainfrom
hyunseong96:isosurf_v

Conversation

@hyunseong96

Copy link
Copy Markdown
Contributor

Hi, I create a PR to add velocity option to isosurface.

I added velocity where the selection of fields are needed.
And I realized that input has m/s unit for velocity, so I made a conversion of unit if a model uses year.

So the test prm file test whether isosurfaces work with a velocity criterion.
Vertical velocity (v) varies by left and right side of the model. left half has 8 m/yr, and right side has 2 m/yr. The rest of the parameters are isotropic.
By the isosurface criterion, refinement levels are 3 for the velocity between 5 and 10 m/yr and 1 for the velocity between 0 and 5 m/yr. I put x and y repetitions as 2.
The result expects nodes to be 2^(1+ isosurfaces velocity level)*(repetitions). The result file has 32 on left side and 8 on right side. Therefore, isosurface velocity option works well.
V

@hyunseong96 hyunseong96 changed the title isosurface for velocity Adding velocity option to isosurfaces refinement strategy Jul 30, 2026

@bangerth bangerth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The velocity is a vector, but you need a scalar to compare with. Would you mind renaming "Velocity" everywhere you mention it in this patch by "Norm of velocity" or norm_of_velocity, etc.?

Otherwise very close!

Comment on lines -382 to 396
"For now, only temperature and compositional fields are allowed as "
"field entries. The key words could be 'Temperature' or one of the names "
"For now, only temperature, velocity and compositional fields are allowed as "
"field entries. The key words could be 'Temperature', 'Velocity' or one of the names "
"of the compositional fields which are either specified by user or set up "
"as C\\_0, C\\_1, etc."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think at the end of this paragraph, it would be useful to state that for velocity (or norm of velocity, see the comment above) the values you compare against are either given in meters per second or meters per year, depending on whether the "Use years instead of seconds" parameter is set.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah yes. They need to be more specified. I will fix so!

@bangerth

Copy link
Copy Markdown
Contributor

/rebuild

@bangerth

Copy link
Copy Markdown
Contributor

Tag me when you're done. Before you do so, please squash everything into one commit!

@hyunseong96
hyunseong96 force-pushed the isosurf_v branch 2 times, most recently from 9097246 to dd359d3 Compare July 30, 2026 21:01
@hyunseong96

Copy link
Copy Markdown
Contributor Author

@bangerth Thanks! Now commits are squashed and I added explanations for norm of velocity

@bangerth

Copy link
Copy Markdown
Contributor

The isosurfaces_simple_box_2D_custom_fieldnames_fail test now fails. Can you investigate what is going wrong?

enum class PropertyType
{
Temperature,
Velocity,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I meant to also rename this to Norm_of_velocity.

Comment thread source/mesh_refinement/isosurfaces.cc Outdated
// The property name has not been found. This could be because the compositional field is not present.
// Abort and warn the user.
std::string key_list = "Temperature";
std::string key_list = "Temperature, Norm of Velocity";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here and perhaps elsewhere, I think I would only capitalize the first letter. So

Suggested change
std::string key_list = "Temperature, Norm of Velocity";
std::string key_list = "Temperature, Norm of velocity";

with corresponding changes where necessary.

@hyunseong96

Copy link
Copy Markdown
Contributor Author

@bangerth Yesss. I checked the prm file. 'isosurfaces_simple_box_2D_custom_fieldnames_fail' prm returns the error message about the available options. The test diff file shows:
- Temperature, field1, FieldB, fIEld_C.
+ Temperature, Norm of Velocity, field1, FieldB, fIEld_C.
Previously, the available options were the temperature and compositions. Now that norm of velocity is added, an error message has changed, and it turned into a test failure. I think it will be solved once I change the name of parameter into "Norm of velocity" and push this line change.
It is sort of interesting that I finally made a PR affecting previous tests!

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.

2 participants