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

Explicit dynamics #14658

Merged
merged 17 commits into from May 19, 2020
Merged

Explicit dynamics #14658

merged 17 commits into from May 19, 2020

Conversation

cbolisetti
Copy link
Contributor

@cbolisetti cbolisetti commented Jan 29, 2020

This is a recreation of PR #12730 and an alternative to the formulation in PR #12628.

This PR demonstrates a formulation of the explicit dynamics solver that does not interfere with the current implementation of ActuallyExplicitEuler. This can be considered option 2 for an explicit dynamics formulation, option 1 being PR #12628 implemented by @hugary1995. This PR is the barebones version of the formulation intended for discussions geared towards a consensus on the design. Currently, the following are included in this PR:

  • A CentralDifference timeintegrator object that derives from ActuallyExplicitEuler and overrides the ComputeTimeDerivatives function to include the second derivative according to the Central Difference formulas.
  • Note that, since the time derivatives and strains, etc. are computed before the solve, the time derivatives should be computed as u_dot = u_old - u_older, and similarly the strains. The derivatives in the CentralDifference timeintegrator are evaluated accordingly. This required access to a solution state that is 'older than older' (current - 3) and @sveerara made changes to framework to provide that access. The framework changes in this PR correspond to this.
  • We also changed inertia kernels (InertialForce, NodalTranslationalInertia, NodalRotationalInertia) so that the inertia kernels are timeintegrator-agnostic and will provide the right residual and Jacobian regardless of the timeintegrator.
  • We also implemented changes in the inertia kernels for lumped mass calculations, so that correct residuals are calculated for lumped mass solves.
  • Tests include 1D, 2D, and 3D problems in the TensorMechanics module. The results match with the corresponding implicit, Newmark-Beta solutions. The formulation works fine with Dirichlet and Periodic BCs. Tests also include lumped mass solves. To benchmark the lumped mass solves, we created identical models with manually lumped masses using nodal kernels. The implicit and explicit results match pretty well here too.
  • While this formulation does not change the current implementation of ActuallyExplicitEuler, it does, however, require that other kernels (inertia kernels and strain kernels, for example) be evaluated differently when CentralDifference timeintegrator is used. This is not ideal and there might be a more elegant way to do this. Therefore, suggestions and ideas are appreciated.
  • I recently discovered that this formulation is not working with Preset BCs. Since Preset and Dirichlet BCs were combined recently, I had to set preset=false in the FunctionDirichletBCs in the test input files to get them working. I am not yet sure why this happens.

Tagging @sveerara @bwspenc @recuero

@moosebuild
Copy link
Contributor

moosebuild commented Jan 29, 2020

Job Documentation on e875638 wanted to post the following:

View the site here

This comment will be updated on new commits.

@loganharbour
Copy link
Member

I've just looked through the framework stuff in an attempt to understand what's going on with the preset issue we discussed today. I would suggest with the solution state access to just point to the solution, old solution, and older solution vectors for the states 0-2, instead of making more vectors and copying. Then add more as necessary for states 3+. It would make sense for solutionState(0 to 2) to work even without sizes set. Or to set the sizes according to which dots are available if the user doesn't set them.

@cbolisetti
Copy link
Contributor Author

cbolisetti commented Jan 30, 2020

I've just looked through the framework stuff in an attempt to understand what's going on with the preset issue we discussed today. I would suggest with the solution state access to just point to the solution, old solution, and older solution vectors for the states 0-2, instead of making more vectors and copying. Then add more as necessary for states 3+. It would make sense for solutionState(0 to 2) to work even without sizes set. Or to set the sizes according to which dots are available if the user doesn't set them.

@loganharbour that makes sense. I need to figure out how to do that though. I'll stop by later and ask you if you don't mind.

@cbolisetti cbolisetti closed this Jan 30, 2020
@cbolisetti cbolisetti reopened this Jan 30, 2020
@sveerara
Copy link
Contributor

sveerara commented Feb 4, 2020

@loganharbour, thanks for reviewing this PR. I agree with your comment on not adding extra vectors for solution states 0 to 2 and instead making it point to the existing vectors for current, old and older solutions.

@cbolisetti, please let me know if you need any help with addressing the comments or if you would like me to make any changes to the framework part. Thanks!

@moosebuild
Copy link
Contributor

Job Precheck on 56de571 : invalidated by @cbolisetti

@loganharbour
Copy link
Member

I've re-written the solution state methods in order to use the current vectors. Only thing I'm stuck on now is if the dot dot, dot old, etc, methods need to be generalized as well. @cbolisetti I've looked around for you today but didn't see you around.

@cbolisetti
Copy link
Contributor Author

I've re-written the solution state methods in order to use the current vectors. Only thing I'm stuck on now is if the dot dot, dot old, etc, methods need to be generalized as well. @cbolisetti I've looked around for you today but didn't see you around.

@loganharbour thats great, thank you. I was busy yesterday. I'll stop by today.

@moosebuild
Copy link
Contributor

moosebuild commented Feb 7, 2020

Job Test timings on 1b37911 wanted to post the following:

View timings here

This comment will be updated on new commits.

@moosebuild
Copy link
Contributor

Job Documentation on 34ab69d : invalidated by @loganharbour

Writing doc timed out again

Copy link
Member

@loganharbour loganharbour left a comment

Choose a reason for hiding this comment

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

Only large concern here is the lack of an interface for accessing the nodal residual values

@loganharbour
Copy link
Member

Took care of merge conflicts. I need a review for the last commit (mine), and we should have a discussion about an interface for the nodal residual values as seen in InertialForce.

@cbolisetti
Copy link
Contributor Author

Took care of merge conflicts. I need a review for the last commit (mine), and we should have a discussion about an interface for the nodal residual values as seen in InertialForce.

Yes, let's talk when you are back next week. One of the tests is failing now though.

@loganharbour
Copy link
Member

One of the tests is failing now though.

Looks unrelated - passing now. Sounds good for next week.

@moosebuild
Copy link
Contributor

All jobs on 1b37911 : invalidated by @loganharbour

Re-try with fixed race conditions

framework/src/systems/SystemBase.C Outdated Show resolved Hide resolved
framework/include/problems/FEProblemBase.h Outdated Show resolved Hide resolved
framework/include/systems/SystemBase.h Outdated Show resolved Hide resolved
@loganharbour loganharbour force-pushed the explicit-dynamics branch 2 times, most recently from a23638e to cfb18bd Compare May 18, 2020 23:30
@moosebuild
Copy link
Contributor

Job Precheck on cfb18bd wanted to post the following:

Your code requires style changes.

A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:

curl -s https://mooseframework.inl.gov/docs/PRs/14658/style.patch | git apply -v

Alternatively, with your repository up to date and in the top level of your repository:

git clang-format 7a584baf0930a4788a9e24043eecdf763240abda

loganharbour
loganharbour previously approved these changes May 19, 2020
Copy link
Member

@loganharbour loganharbour left a comment

Choose a reason for hiding this comment

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

My approval for the commits that aren't mine (modules, modules doco, etc).

@moosebuild
Copy link
Contributor

Job Private App tests on e875638 : invalidated by @loganharbour

Copy link
Member

@loganharbour loganharbour left a comment

Choose a reason for hiding this comment

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

My approval for the commits that aren't mine.

@hugary1995
Copy link
Contributor

Should CentralDifference and NewmarkBeta stay in framework or in tensor_mechanics?
I am asking because it seems to me that if you don't modify the residual of the second time derivative kernel you won't be able to use those time integrators anyways.
Another way to look at this is CentralDifference and NewmarkBeta are in framework but all regression tests are in tensor_mechanics...

@loganharbour
Copy link
Member

Should CentralDifference and NewmarkBeta stay in framework or in tensor_mechanics?

I'm fine with moving it - that's probably the best bet here. This has sat around for far too long, I'm going to merge now and will put up a smaller PR for that plus a few other things.

@loganharbour loganharbour merged commit 51412f3 into idaholab:next May 19, 2020
@hugary1995
Copy link
Contributor

Okay that makes sense. Thanks to you all @cbolisetti @sveerara @loganharbour I really appreciate all your work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants