Skip to content

Miscellaneous bugfixes and cleanup#475

Open
brbass wants to merge 5 commits into
llnl:developfrom
brbass:brody/bugfixes
Open

Miscellaneous bugfixes and cleanup#475
brbass wants to merge 5 commits into
llnl:developfrom
brbass:brody/bugfixes

Conversation

@brbass
Copy link
Copy Markdown
Contributor

@brbass brbass commented Mar 4, 2026

Summary

  • Removes the old, non-functional volume calculation from SPH and requests Voronoi cells from RK instead.
  • Adds missing includes across several files.
  • Moves NodeIteratorBase stream operators from the inline header to the source file.
  • Fixes the Johnson-Cook damage model to avoid circular dependencies
  • Adds parallel GenerateRatioSphere in 2D.
  • Includes material internal energy in the generic node distribution.

ToDo :

  • Annotate RELEASE_NOTES.md with notable changes.
  • Create LLNLSpheral PR pointing at this branch. (PR#)
  • LLNLSpheral PR has passed all tests.

@brbass brbass mentioned this pull request Mar 26, 2026
3 tasks
@mcfadden8
Copy link
Copy Markdown
Collaborator

build_fail.txt

Hi @brbass - Our CI is reporting a build failure (attached snippet). Have you seen this before?

@brbass
Copy link
Copy Markdown
Contributor Author

brbass commented Apr 30, 2026

build_fail.txt

Hi @brbass - Our CI is reporting a build failure (attached snippet). Have you seen this before?

Fixed! I think it is probably a difference of compilers since this warning didn't cause an error in my local build.

@jmikeowen
Copy link
Copy Markdown
Collaborator

We have some CI builds enforcing warnings as errors just to keep things clean, so that's probably the difference.

Comment thread src/KernelIntegrator/FlatConnectivity.cc Outdated
JohnsonCookDamagePolicy():
UpdatePolicyBase<Dimension>({SolidFieldNames::flaws,
SolidFieldNames::plasticStrain}) {
UpdatePolicyBase<Dimension>() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why remove these dependencies? I believe this policy does depend on both the flaws and plastic strain -- was there a circular dependency resulting?

Also as an aside the JC damage and strength are not well tested, so be cautious using them.

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.

Comment thread src/FSISPH/SolidFSISPH.cc
Comment thread src/SPH/SolidSPH.cc

// Call the ancestor.
SPHBase<Dimension>::initializeProblemStartupDependencies(dataBase, state, derivs);
SPH<Dimension>::initializeProblemStartupDependencies(dataBase, state, derivs);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This one is a C++ question for me: the SPH class does not have an initializeProblemDependencies method, but it's ancestor (SPHBase) does. What happens by calling the SPH:: declaration here?

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.

If there is no SPH::declaration, then it will call SPHBase::declaration. But if there is an SPH::declaration in the future, this will call that instead. Seems like an awful bug to track down, so this is just being nice to our future selves.

@mcfadden8
Copy link
Copy Markdown
Collaborator

Hi @brbass, we are hopeful to get this into our next release. I'm ready to take any changes you might have in response to the questions/comments raised in this PR. If no changes are required, please let me know that as well.

Thanks!

@brbass
Copy link
Copy Markdown
Contributor Author

brbass commented May 11, 2026

Hi @brbass, we are hopeful to get this into our next release. I'm ready to take any changes you might have in response to the questions/comments raised in this PR. If no changes are required, please let me know that as well.

Thanks!

Responded to all! Feel free to merge whenever you guys think it is ready.

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