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 Libgmsh support/ flboundadapt #168

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

Add Libgmsh support/ flboundadapt #168

wants to merge 9 commits into from

Conversation

jrper
Copy link
Contributor

@jrper jrper commented Oct 12, 2017

This change set gives the option to use functions from the libgmsh library to read and write .msh files and to read a .geo file directly. It also introduces a new tool flboundadapt which generates a .msh file from a .geo file using fluidity metric information taken from a .flml file (eg. a checkpoint file.)

errormetric

testmsh

@jrper jrper requested a review from stephankramer October 12, 2017 14:49
call calculate_diagnostic_variables_new(states)

! Find the external mesh field
!call find_mesh_field_to_adapt(states(1), old_mesh_field)
Copy link
Contributor

Choose a reason for hiding this comment

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

Commented out code.
Should this perhaps be topology_mesh_name ? The metric doesn't necessarily live on the CoordinateMesh: periodic, extruded, higher order, etc. - I realize not all of these combination may actually work with this tool of course. If Coordinate is the safest choice here, maybe add some comment why you think that is...

configure.in Outdated
@@ -1067,7 +1098,7 @@ fi

#####
#
LIBS="$LIBS -L./lib"
LIB="$LIBS -L./lib"
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks wrong

@@ -1337,4 +1337,25 @@ function extract_prognostic_velocity(state, stat) result(vfield)

end function extract_prognostic_velocity

subroutine get_surface_ids(bc_path, mesh, surface_ids)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a one line comment saying exactly what this routine does (or more if required).

@@ -1097,6 +1102,10 @@ function make_mesh (model, shape, continuity, name) &
size(mesh%faces%surface_node_list), name='Surface'//trim(mesh%name))
#endif
end if
if (associated(model%surface_names)) then
allocate(mesh%surface_names(size(model%surface_names)))
mesh%surface_names(:) = model%surface_names(:)
Copy link
Contributor

Choose a reason for hiding this comment

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

Pedantic: spurious (:)

@@ -4199,4 +4200,39 @@ subroutine write_minmax_tensor(tfield, field_expression)

end subroutine write_minmax_tensor

subroutine set_surface_names(mesh, names, ids)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here: "docstring" please

end subroutine set_surface_names

subroutine get_surface_ids_from_names(mesh, names, ids)
type(mesh_type), intent(in) :: mesh
Copy link
Contributor

Choose a reason for hiding this comment

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

And here

subroutine copy_c_string_to_fortran(c_string, f_string)

type(c_ptr), intent(in), target :: c_string
character(len=*) :: f_string
Copy link
Contributor

Choose a reason for hiding this comment

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

intent

call c_f_pointer(c_string, arr, [strlen(c_string)])
!!! make into a pointer to a Fortran string
call wrap_to_string(strlen(c_string), arr, f_ptr)
!!! copy happens here
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should add a check that len(f_string)<=len(c_string)

@stephankramer
Copy link
Contributor

Some more queries on the new functionality to specify a .geo file as the input mesh:

  • in this case is there a metric involved? - or do I get the same as if I would run gmsh -2 or gmsh -3 on that .geo file - also what are the options for meshing algorithm and optimisation
  • if I do have a .geo file as input - what happens if I checkpoint? Is the .msh file written out and the options tree changed to point to that .msh file?

@jrper
Copy link
Contributor Author

jrper commented Oct 13, 2017

To address the metaqueries:

  • The current version uses the lengths exactly as specified in the .geo, with a metric only being used in flboundadapt tool. There would probably be a use case of allowing the option to use a metric with "adapt at first tilmestep" or similar, but that would have required a slightly wider set of changes to the code to still do the Right Thing(TM) in terms of your second question.
  • It's set up to do the same thing with .geo files that we already do with ExodusII format. Just as you surmised, we only write .msh files, and the mesh format in the options tree is changed to gmsh.

@drhodrid
Copy link
Contributor

Just a minor comment from me - would it be possible to update the manual to reflect these changes?

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