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

Change material model logic for determining initial time step number #2529

Closed

Conversation

naliboff
Copy link
Contributor

Related to #2362.

@bangerth - Let me know if this is what you had in mind. A similar logic was already in drucker_prager.

diffusion_dislocation did not have a reference to the tilmestep at all in setting the strain rate during time step 0 or initiation stage. Rather, it just checks in.strain_rate.size() before calling a function to compute the viscosity.

@tjhei
Copy link
Member

tjhei commented Jun 28, 2018

Can we create a function in SimulatorAccess instead? Maybe something like is_initial_time_step()?

@bangerth
Copy link
Contributor

Exactly what I had in mind. Unfortunately, this now fails several tests -- but I can't see why because @tjhei 's server doesn't want us to see the output diffs :-(

@bangerth
Copy link
Contributor

There is also a merge conflict you'll have to address.

@naliboff
Copy link
Contributor Author

@tjhei - I think having that something like is is_initial_time_step() is a good idea. Would this apply to only after the model setup or during the first set of set of solves (e.g., time step "0" now)? Until then, any objections to the logic introduced here?

@bangerth - I think I know what the issue is with the tests. I'll rebase and see if that fixes the problem.

@naliboff naliboff force-pushed the initial_timestep_number branch 3 times, most recently from 3154f31 to 3671904 Compare June 29, 2018 03:35
@naliboff
Copy link
Contributor Author

Ok, got it down to two tests failing. @tjhei - When you have a chance, can you take a look at the tester permissions? The tests diffs are currently not accessible.

@tjhei
Copy link
Member

tjhei commented Jun 29, 2018

The tests diffs are currently not accessible.

that means that the upload to the webserver hasn't completed yet. It is available now but it might take up to an hour to complete.

@bangerth
Copy link
Contributor

bangerth commented Jun 30, 2018 via email

@gassmoeller
Copy link
Member

I do not think we need to store a separate flag, we could just move the logic that is already present in many plugins into a separate function SimulatorAccess::is_during_initialization would then check for timestep_number == numbers::invalid_unsigned_int. I am sure this is used in a number of plugins (at least in some of my own that are not in master).

@tjhei
Copy link
Member

tjhei commented Jul 3, 2018

I do not think we need to store a separate flag

agreed.

@bangerth
Copy link
Contributor

@naliboff and @anne-glerum: We discussed this before :-) I just looked and there is no such is_during_initialization() function. I'll add one next.

@bangerth
Copy link
Contributor

@naliboff -- #2972 was just merged. Want to give that a try instead of the approach you have here?

@naliboff
Copy link
Contributor Author

@bangerth 👍

@naliboff
Copy link
Contributor Author

@MFraters - The visco_plastic Newton derivative tests are failing with the proposed changes. Can we take a look at this together at some point?

@MFraters
Copy link
Member

hmm, this is interesting. So I think the reason it is failing has to do with how the test is set up. For the test I needed to trick the material model in using not the reference strain-rate, but the strain-rate which I provided in the test. So we need a way to set this->simulator_is_past_initialization() to true for the test.

@naliboff
Copy link
Contributor Author

oofta ;) Is the strain-rate in the test constant throughout the model domain?

@MFraters
Copy link
Member

no, I have a different strain-rate tensor for each 'quadrature' point.

@MFraters
Copy link
Member

hmm, I will have to look into this a bit more tomorrow. Adding a function to set the variable simulator_is_past_initialization through simulator access wont work because it only has a const pointer to simulator (a pointer which can't change the simulator class). So it would have to be a function directly in the simulator class which I would somehow have to get access to in the file tests/visco_plastic_derivatives_2d.cc. This feels quite intrusive, so I do need to think about it a bit more. Help with this problem is greatly appreciated.

@bangerth
Copy link
Contributor

Can you explain why the test now fails? Are you saying that the branch you end up in in the computation of e_dot_ii is wrong?

@MFraters
Copy link
Member

yes, it should compute second invariant of the strain-rate tensor instead of using the reference strain-rate.

@naliboff
Copy link
Contributor Author

naliboff commented Apr 9, 2020

@MFraters - Any chance we can come back and take a look at how to fix these tests?

As I understand it, the issue is that current logic in visco_plastic.cc allows one to calculate the standard strain rate invariant prior to initialization. With the change in this PR, we not only use the reference strain rate on the first iteration of time step 0, but now also before initialization is finished. This then breaks the derivative tests as it passes visco_plastic a strain rate tensor prior to initialization.

Could we somehow trick simulator_is_past_initialization() to return true just for this test?

@naliboff
Copy link
Contributor Author

naliboff commented Aug 13, 2020

Closing this as there does not seem to be an easy path for adjusting the Newton test setup.

@naliboff naliboff closed this Aug 13, 2020
@MFraters
Copy link
Member

ah, sorry. I totally missed your last reply. If you still want to discuss this at some point or want me to have a look at it again let me know.

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.

None yet

5 participants