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

New generalized granular pair style #1342

Merged
merged 50 commits into from Mar 29, 2019

Conversation

@dsbolin
Copy link
Collaborator

dsbolin commented Feb 19, 2019

This is a new granular pair style that expands options for granular contact models

This includes options for cohesion, rolling and twisting friction, and assignment of different coefficients to different atom types via pair_coeff command (new compared to existing pair gran/* styles).
Author(s)
Dan S. Bolintineanu, Ishan Srivastava, Jeremy B. Lechman

Licensing

By submitting this pull request, I agree, that my contribution will be included in LAMMPS and redistributed under the GNU General Public License version 2.

Please complete the following statement by adding "yes" or "no":
My contribution may be re-licensed as LGPL (for use of LAMMPS as a library linked to proprietary software):

yes

Backward Compatibility

Please state whether any changes in the pull request break backward compatibility for inputs, and - if yes - explain what has been changed and why

N/A

Implementation Notes

Provide any relevant details about how the changes are implemented, how correctness was verified, how other features - if any - in LAMMPS are affected

Should not affect any other features in lammps

Post Submission Checklist

Please check the fields below as they are completed after the pull request has been submitted

  • The feature or features in this pull request is complete
  • Licensing information is complete
  • Corresponding author information is complete
  • The source code follows the LAMMPS formatting guidelines
  • Suitable new documentation files and/or updates to the existing docs are included
  • The added/updated documentation is integrated and tested with the documentation build system
  • The feature has been verified to work with the conventional build system
  • The feature has been verified to work with the CMake based build system
  • A package specific README file has been included or updated
  • One or more example input decks are included

Dan Stefan Bolintineanu and others added some commits Apr 16, 2018

dsbolin
Added type- and form-dependent conditionals in PairGranular::compute,
still some issues with overall structure of the code.
dsbolin
Moved all model option syntax for pair granular to pair coeff command;
added global cutoff option for pair style granular command; initial
write-up of documentation.
Dan Stefan Bolintineanu
Fixed tangential damping in pair granular; fixed order of template ar…
…guments, so that pair gran and gran/multi now produce identical results for same settings (as they should)
Dan S. Bolintineanu
Several changes to new consolidated granular code:
- Normal contact models take Young's modulus and Poisson's ratio (instead of Young's and shear modulus)
- Mixing of Young's moduli corrected
- Changes to cutoffs corrected for JKR pulloff distance
- Renamed 'mindlin' to 'linear_history'
- Progress on doc page
Dan S. Bolintineanu
More changes to pair granular:
- tangential damping now set by scaling the normal damping
- some fixes to the twisting coefficients for the Marshall twist model
- progress (completion?) of doc page
Dan S. Bolintineanu
Added generalized granular option to fix wall/gran and fix wall/gran/…
…region; some minor bug fixes for pair granular
Dan S. Bolintineanu
Changes to new generalized granular pair styles and fix wall/gran
-Clean-up of unused variables in code
-Bug fix for single method of pair granular
-Changes to fix wall/gran to fix issues with JKR
-Doc page updates for fix wall/gran and fix wall/gran/region

@dsbolin dsbolin requested a review from sjplimp as a code owner Feb 19, 2019

@rbberger rbberger added the needs_work label Feb 20, 2019

@akohlmey
Copy link
Member

akohlmey left a comment

You must not commit files in the src folder to git, that are installed a package and reside in the corresponding package folder (example: pair_granular.h). In fact, the names of such files should be added to .gitignore instead, so they won't get added/committed by accident.

akohlmey added some commits Mar 15, 2019

@akohlmey

This comment has been minimized.

Copy link
Member

akohlmey commented Mar 15, 2019

@dsbolin I think with my added changes, this is ready to be included. I've decided to remove the USER-OMP version of fix wall/gran rather than porting it to the updated code.
It would be nice if there were a few small/fast examples for this addition included, if only as a reference for testing future modifications. Those examples could be provided in a separate pull request.

@dsbolin

This comment has been minimized.

Copy link
Collaborator Author

dsbolin commented Mar 18, 2019

@akohlmey Thanks for looking at this. I will add a few examples in a new pull request in the near future.

must not approve when also doing the merge as per github workflow docs

@sjplimp sjplimp self-assigned this Mar 26, 2019

@sjplimp

This comment has been minimized.

Copy link
Contributor

sjplimp commented Mar 27, 2019

@akohlmey @dsbolin Looked thru the files, made only reformatting changes. So everything
should still work, though @dsbolin it wouldn't hurt to re-run your examples. 3 Qs

  1. @akohlmey do you want some of the constants at the top of pair_granular.cpp
    to be refs to math_const.h
  2. These pair styles do not invoke ev_init(), including the new pair_granulat.cpp, like the recent change. Not sure if that will get merged for the other modified pair styles.
  3. I'm unclear on why the variable nondefault_history_transfer appears in both pair_granular.h and pair_gran_hooke_history.h. I think it should only be in the latter?
    Otherwise good to go so far as I see. Great work Dan!
@sjplimp

This comment has been minimized.

Copy link
Contributor

sjplimp commented Mar 27, 2019

@dsbolin Just realized there are no example scripts included. Do you think
some should be added to the examples dir? There is a pour sub-dir now.
It could be renamed to gran or granular, and new files with additional pair granular
options enabled could be added? Note there is a granregion as well with your
examples. A script with the new options could be added there also.

@akohlmey

This comment has been minimized.

Copy link
Member

akohlmey commented Mar 28, 2019

@dsbolin Just realized there are no example scripts included. Do you think
some should be added to the examples dir? There is a pour sub-dir now.
It could be renamed to gran or granular, and new files with additional pair granular
options enabled could be added? Note there is a granregion as well with your
examples. A script with the new options could be added there also.

this was discussed already. a second pull request with examples is in preparation.

@akohlmey

This comment has been minimized.

Copy link
Member

akohlmey commented Mar 28, 2019

@akohlmey @dsbolin Looked thru the files, made only reformatting changes. So everything
should still work, though @dsbolin it wouldn't hurt to re-run your examples. 3 Qs

  1. @akohlmey do you want some of the constants at the top of pair_granular.cpp
    to be refs to math_const.h

working on it now.

  1. These pair styles do not invoke ev_init(), including the new pair_granulat.cpp, like the recent change. Not sure if that will get merged for the other modified pair styles.

the pull request predates that change, so they will not show up here. however, pair granular needs to be updated and i am doing this right now.

  1. I'm unclear on why the variable nondefault_history_transfer appears in both pair_granular.h and pair_gran_hooke_history.h. I think it should only be in the latter?

i don't see it in pair_granular.h but in pair.h. it should be only in pair.h then, and also initialized there to avoid uninitialized access and thus random crashes. i see another possible uninitialized data (which may have gone unnoticed, since on linux data is initialized for the first time typically to 0 due to how its malloc() implementation works).

will make those changes and some more cosmetic cleanup.

Otherwise good to go so far as I see. Great work Dan!

akohlmey added some commits Mar 28, 2019

@akohlmey

This comment has been minimized.

Copy link
Member

akohlmey commented Mar 28, 2019

@sjplimp this PR should now be ready to be merged

@akohlmey akohlmey merged commit 30929d9 into lammps:master Mar 29, 2019

6 checks passed

lammps/pull-requests/build-docs-pr head run ended
Details
lammps/pull-requests/cmake/cmake-serial-pr head run ended
Details
lammps/pull-requests/kokkos-omp-pr head run ended
Details
lammps/pull-requests/openmpi-pr head run ended
Details
lammps/pull-requests/serial-pr head run ended
Details
lammps/pull-requests/shlib-pr head run ended
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.