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

Virial stress #137

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

Virial stress #137

wants to merge 38 commits into from

Conversation

kinanbezem7
Copy link

@kinanbezem7 kinanbezem7 commented Oct 17, 2024

Added virial stress to particles.hpp and the HDF5 output for the pmb model.
Added logic for calculating virial stress in compute full energy in force_pmb.hpp.
Compiles and runs kalthoff winkler, however the virial stress in the output is zero.

I think the forces may not be being calculated properly in the energy logic. I'm also unsure of how to handle bond breakage here, but my entire approach may be wrong so this might be totally unnecessary

Copy link
Collaborator

@streeve streeve left a comment

Choose a reason for hiding this comment

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

First review - let's get it working without fracture first

bfg.jar Outdated Show resolved Hide resolved
src/force/CabanaPD_Force_PMB.hpp Outdated Show resolved Hide resolved
src/force/CabanaPD_Force_PMB.hpp Outdated Show resolved Hide resolved
src/force/CabanaPD_Force_PMB.hpp Outdated Show resolved Hide resolved
kinanbezem7 and others added 3 commits October 18, 2024 11:09
…accordingly so we do not recalculate force, needed to add a const version of sliceForce() to avoid compiling errors, virial stress output is still zero for everything.
Copy link
Collaborator

@streeve streeve left a comment

Choose a reason for hiding this comment

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

Zeros are because the only implemented stress is for Elastic (no fracture) with PMB, which requires modifying the elastic wave problem to use PMB. Looks good on my end making that change. Let me know if you have questions about adding this for fracture

.vscode/settings.json Outdated Show resolved Hide resolved
src/CabanaPD_Particles.hpp Outdated Show resolved Hide resolved
src/force/CabanaPD_Force_PMB.hpp Outdated Show resolved Hide resolved
@kinanbezem7
Copy link
Author

I've made the suggested changes and it is now working with non-fracture problems using the PMB method. (I may have pushed an altered version of elastic wave problem that I used to test it...please disregard this). Will move on to implementing virial stress with fracture.

@streeve
Copy link
Collaborator

streeve commented Oct 18, 2024

I've made the suggested changes and it is now working with non-fracture problems using the PMB method. (I may have pushed an altered version of elastic wave problem that I used to test it...please disregard this). Will move on to implementing virial stress with fracture.

Sounds good. One thing that will help quite a bit is using pre-commit, see: https://github.com/ORNL/CabanaPD/blob/main/CONTRIBUTING.md It will basically stop you from committing certain unintended things (unformatted code, merge conflicts, etc.)

One other note - we'll need to remove the comments tracking incremental changes before we merge (the fact that the force slice was moved outside the kernel is tracked in the git history and would likely be more confusing than not in the future, for example)

@streeve streeve mentioned this pull request Oct 18, 2024
10 tasks
@kinanbezem7
Copy link
Author

1-s2.0-S0020768322000427-main.pdf

Added virial stress to pmb with fracture. Could you please doublecheck the volume summation. it should be normalised by the total volume of the horizon regardless of the number of points within the horizon. I'm thinking we might need to adjust the non-fracture volume too as it does not include vol(i) in the division.

@streeve
Copy link
Collaborator

streeve commented Oct 22, 2024

1-s2.0-S0020768322000427-main.pdf

Added virial stress to pmb with fracture. Could you please doublecheck the volume summation. it should be normalised by the total volume of the horizon regardless of the number of points within the horizon. I'm thinking we might need to adjust the non-fracture volume too as it does not include vol(i) in the division.

@pabloseleson can you take a look?

Copy link
Collaborator

@streeve streeve left a comment

Choose a reason for hiding this comment

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

Should we add this for LPS as well? Or wait for now?

.gitignore Outdated Show resolved Hide resolved
examples/mechanics/elastic_wave.cpp Outdated Show resolved Hide resolved
src/CabanaPD_Particles.hpp Outdated Show resolved Hide resolved
@pabloseleson
Copy link
Collaborator

Which equation is being implemented exactly?

@kinanbezem7
Copy link
Author

Which equation is being implemented exactly?

Eq. 29.

@pabloseleson
Copy link
Collaborator

pabloseleson commented Nov 19, 2024

@kinanbezem7 : I looked at Eq. (29) in the reference here. There is something awkward in that equation. The RHS of the equation is for the stress tensor at X. However, the LHS of the equation doesn't depend on X.

Instead, I looked at an alternative reference here and implemented Eq. (24) of that reference. In this reference, there is a dependence on R on both the RHS and LHS, so it makes more sense to me.

BTW: this stress tensor is in general nonsymmetric (like the classical Piola stress tensor, see here). So, it seems we would need to compute the full tensor, not only the 6 entries. I only included the 6 entries for now.

@pabloseleson
Copy link
Collaborator

@streeve : Please take a look at the current implementation.

src/force/CabanaPD_Force_PMB.hpp Outdated Show resolved Hide resolved
src/force/CabanaPD_Force_PMB.hpp Outdated Show resolved Hide resolved
@streeve streeve mentioned this pull request Nov 22, 2024
Copy link
Collaborator

@streeve streeve left a comment

Choose a reason for hiding this comment

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

Couple small things to clean up, but this is close to ready. @kinanbezem7 let me know if it makes sense how to fix the failing test or remaining comments or we can look at it in person

src/force/CabanaPD_Force_PMB.hpp Outdated Show resolved Hide resolved
src/force/CabanaPD_Force_PMB.hpp Outdated Show resolved Hide resolved
src/force/CabanaPD_Force_PMB.hpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@streeve streeve left a comment

Choose a reason for hiding this comment

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

Missing LPS stresses

src/force/CabanaPD_Force_PMB.hpp Outdated Show resolved Hide resolved
src/force/CabanaPD_Force_PMB.hpp Show resolved Hide resolved
@streeve
Copy link
Collaborator

streeve commented Dec 30, 2024

@pabloseleson can you take a look at the new example - the crack isn't centered. @kinanbezem7 there are a number of new files that look unintentional

@kinanbezem7
Copy link
Author

@pabloseleson can you take a look at the new example - the crack isn't centered. @kinanbezem7 there are a number of new files that look unintentional

Apologies, I got CabanaPD working on WSL on windows and then tried to work from there for this last part, so probably a bunch of files got thrown in. Still getting used to Github

@streeve
Copy link
Collaborator

streeve commented Dec 30, 2024

@pabloseleson can you take a look at the new example - the crack isn't centered. @kinanbezem7 there are a number of new files that look unintentional

Apologies, I got CabanaPD working on WSL on windows and then tried to work from there for this last part, so probably a bunch of files got thrown in. Still getting used to Github

No worries, but avoidinggit add . will help. This branch should work again after the merge earlier today

@kinanbezem7
Copy link
Author

@streeve Is there anything else that you need me to do with this?

@streeve
Copy link
Collaborator

streeve commented Jan 6, 2025

@streeve Is there anything else that you need me to do with this?

I had hoped @pabloseleson would have time to take a look. Instead, you could remove the unintended files, remove the new example, and run the crack branching case with stress output posted here

@kinanbezem7
Copy link
Author

@streeve Is there anything else that you need me to do with this?

I had hoped @pabloseleson would have time to take a look. Instead, you could remove the unintended files, remove the new example, and run the crack branching case with stress output posted here

I noticed there was a mistake with the LinearLPS stress which I corrected and removed the extra files. I tested this for both LPS and PMB and looks like we are getting reasonable output, but I haven't looked into a proper validation of the magnitude of the stresses

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