Skip to content

Graveyard updates#855

Merged
gonuke merged 9 commits into
svalinn:developfrom
pshriwise:graveyard_updates
Mar 21, 2023
Merged

Graveyard updates#855
gonuke merged 9 commits into
svalinn:developfrom
pshriwise:graveyard_updates

Conversation

@pshriwise

@pshriwise pshriwise commented Feb 13, 2023

Copy link
Copy Markdown
Member

Description

These changes update some behavior of the DagMC::create/remove_graveyard function.

create_graveyard

  • create_graveyard can now create a graveyard without acceleration data structures using the vertices of the model to create a bounding box as a basis for the graveyard volume.
  • create_graveyard will create an implicit complement if none exists so the graveyard volume can be inserted into the model topology correctly.
  • create_graveyard will only (re)construct acceleration data structures for the new graveyard volume and the implicit complement if these data structures were present when the method was initially called.

remove_graveyard

There's a bug when calling create_graveyard with overwrite == true when a graveyard is not present. Previously, this would fail when trying to remove a graveyard that isn't present. This adds a check at the beginning of remove_graveyard to simply return MB_SUCCESS if no graveyard is present.

@gonuke gonuke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @pshriwise - just a couple of maintainability questions

Comment thread src/dagmc/DagMC.cpp
Comment thread src/dagmc/DagMC.cpp Outdated
@makeclean

Copy link
Copy Markdown
Contributor

Should we ever replace an existing graveyard? Should we just not add another to the set?

@pshriwise

pshriwise commented Feb 13, 2023

Copy link
Copy Markdown
Member Author

Should we ever replace an existing graveyard? Should we just not add another to the set?

I can see someone wanting to replace a spherical shell graveyard in case they're learning DAGMC and didn't realize how many triangles this might add to their model without having to re-tessellate the entire thing. Granted, some kind of utility program in the repo would be better than writing it yourself own, which is what they'd have to now.

@gonuke

gonuke commented Feb 16, 2023

Copy link
Copy Markdown
Member

I'm ready to merge but want to give @makeclean a chance to either withdraw or amplify his concern?

@gonuke gonuke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Only one typo in a comment....

Comment thread src/dagmc/DagMC.cpp Outdated
Co-authored-by: Paul Wilson <paul.wilson@wisc.edu>
@gonuke

gonuke commented Mar 21, 2023

Copy link
Copy Markdown
Member

Thanks @pshriwise

@gonuke gonuke merged commit a3f5936 into svalinn:develop Mar 21, 2023
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