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 mesh partitioning tool #1213

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

Conversation

samuelpmishLLNL
Copy link
Contributor

This PR comes from a request by @sethwatts on behalf of the digital twins project. It adds a new command line program that reads in an mfem mesh, partitions it into some number of pieces, and writes out those pieces to separate files.

@samuelpmishLLNL
Copy link
Contributor Author

/style

Comment on lines +25 to +26
add_executable(partitioner partitioner.cpp)
target_link_libraries(partitioner PUBLIC serac_mesh)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
add_executable(partitioner partitioner.cpp)
target_link_libraries(partitioner PUBLIC serac_mesh)
blt_add_executable(NAME partitioner
SOURCES partitioner.cpp
DEPENDS_ON serac_mesh)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what's the difference?

Copy link
Member

Choose a reason for hiding this comment

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

In this case, not much but it keeps it consistent with the rest of the code.

@white238
Copy link
Member

Should we have a "tools" directory for things of this flavor? I think this is the first one.

@white238 white238 closed this Aug 21, 2024
@white238 white238 reopened this Aug 21, 2024
@white238
Copy link
Member

wrong button sorry.

@samuelpmishLLNL
Copy link
Contributor Author

I don't understand why, but one of the CI jobs is concluding that this code is unsafe because of a null pointer dereference(?):

  std::vector< uint32_t > blocks(num_blocks + 1);
  blocks[0] = 0;

Any idea how to work around this? It seems like a false positive to me.

@white238
Copy link
Member

I don't understand why, but one of the CI jobs is concluding that this code is unsafe because of a null pointer dereference(?):

  std::vector< uint32_t > blocks(num_blocks + 1);
  blocks[0] = 0;

Any idea how to work around this? It seems like a false positive to me.

I agree that seems to be a false positive. Have you tried converting num_blocks to std::size_t? Possibly even adding an error check against num_blocks being less than 0 even though that cannot happen.

@white238
Copy link
Member

/style

@tupek2
Copy link
Collaborator

tupek2 commented Aug 27, 2024

Where does this executable live? How does one know where to get it. I do thing something like an tool, or utilities, or apps directory for these kinds of things might make sense.

@samuelpmish
Copy link
Contributor

Where does this executable live? How does one know where to get it.

I believe CMake puts executables in CMAKE_RUNTIME_OUTPUT_DIRECTORY, so it's currently written to /path/to/serac/build_dir/bin, but I think a user could override that to specify a different location.

I do think something like an tool, or utilities, or apps directory for these kinds of things might make sense.

I agree, although I think it makes more sense to worry about organization if/when we have more than 1.

@tupek2 tupek2 self-requested a review August 27, 2024 17:06
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.

4 participants