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

added elastic stress contribution to stress and shear stress vis pp #4001

Merged

Conversation

EstherHeck
Copy link
Contributor

@EstherHeck EstherHeck commented Mar 17, 2021

While the elastic contribution is added to the principal stress postprocesor, it was not in the stress and shear stress postprocesors.

Before your first pull request:

For all pull requests:

For new features/models or changes of existing features:

  • I have tested my new feature locally to ensure it is correct.
  • I have created a testcase for the new feature/benchmark in the tests/ directory.
  • I have added a changelog entry in the doc/modules/changes directory that will inform other users of my change.

@EstherHeck
Copy link
Contributor Author

Following PR #4002 the stress field names needed to add the elastic contribution will have to be changed in stress.cc and shear_stress.cc according to the new naming scheeme from stress_xx to ve_stress_xx etc.

@MFraters
Copy link
Member

Thanks updating these plugins. I think it would be good to explicitly talk about this in the documentation. Could you add a line to the documentation in at the bottom of the cc file and the header file stating that the elastic stress is accounted for if present?

@EstherHeck EstherHeck force-pushed the add-elasticity-to-stress-postprocessor branch from 3b316d9 to bd01f11 Compare March 18, 2021 09:19
@EstherHeck
Copy link
Contributor Author

Good point @MFraters, done!
I dont know why the timestepping commit is shown here now though....

@MFraters
Copy link
Member

I dont know why the timestepping commit is shown here now though....

That should be easy to fix with the following: checkout master, pull from the master from this repository (the default name is origin, so that would be git pull origin master), go back to this branch and do a git rebase -i master and make sure that the last commit before this one is the merge from Timo (with hash 274f11f). Let me know if this works.

@EstherHeck EstherHeck force-pushed the add-elasticity-to-stress-postprocessor branch from bd01f11 to 5ffdb63 Compare March 18, 2021 10:40
@EstherHeck
Copy link
Contributor Author

Thanks @MFraters this worked!

@MFraters
Copy link
Member

Great :)

Can you indent the code? Then we can see the test results. You can do this by using make indent or ninja indent and commit the changes.

If it gives an error you need to make sure that you have installed the correct version of astyle (you can for example use mkdir astyle && cd astyle && wget 'https://sourceforge.net/projects/astyle/files/astyle/astyle 2.04/astyle_2.04_linux.tar.gz' && tar -zxvf astyle_2.04_linux.tar.gz && cd astyle/build/gcc && make && sudo make install).

P.S. could you change [ X] to [X] in the opening post? then it should show checkboxes with checks in them :)

@EstherHeck EstherHeck force-pushed the add-elasticity-to-stress-postprocessor branch from 5ffdb63 to 4d2759b Compare March 18, 2021 11:19
@EstherHeck
Copy link
Contributor Author

Done :)

Copy link
Member

@MFraters MFraters left a comment

Choose a reason for hiding this comment

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

Thanks for updating this these files and adding the documentation!

Apparently we don't have any tests which use either of these visualization postprocessors and elasticity. That might be something to add in the future.

@MFraters MFraters merged commit 384b51e into geodynamics:master Mar 18, 2021
@EstherHeck EstherHeck deleted the add-elasticity-to-stress-postprocessor branch March 25, 2021 07:28
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.

2 participants