-
Notifications
You must be signed in to change notification settings - Fork 240
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
Diffusion [non-local plasticity is still in progress] #2532
base: main
Are you sure you want to change the base?
Conversation
…onto compositional fields
edcd64b
to
edfa6c1
Compare
@alarshi, you mentioned you wanted to push some commits to this pull request before I should look at this? Or is this the state where you had the problem with the compositional field that you want to advect being solved correctly only once? |
…s along with a test file.
@jdannberg Thank you for looking into it! I pushed the material model i modified from visco plastic: diffused plastic. It is exactly the same but has only copy outputs and not plastic additional outputs as the named additional outputs. The parameter file has a field called 'seed' which is not solved after the first time step and I am not sure how to fix it. Thanks again! :) |
source/simulator/helper_functions.cc
Outdated
@@ -1728,6 +1728,107 @@ namespace aspect | |||
} | |||
|
|||
|
|||
template <int dim> | |||
void Simulator<dim>::interpolate_material_output_into_field () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the problem is that this function goes through every compositional field and interpolates the copy outputs onto all of these fields. I think what you really want to do here is instead of looping over all the fields (below), you want to pass this function the compositional field that you want to interpolate your outputs onto as an argument. Then, in solver_schemes.cc, you could call interpolate_material_output_into_field(c)
, and then is this function the interpolation would only be done for that specific field.
source/simulator/helper_functions.cc
Outdated
} | ||
|
||
// put the final values into the solution vector | ||
for (unsigned int c=0; c<introspection.n_compositional_fields; ++c) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the place I was talking about above. All the fields are copied, and not just the ones you want. So here and in line 1804, you want to remove the loop (and c will just be a function argument).
include/aspect/simulator.h
Outdated
* This function is implemented in | ||
* <code>source/simulator/helper_functions.cc</code>. | ||
*/ | ||
void interpolate_material_output_into_field (); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Edit: I guess what I really wanted to say was:
So you want to change that here too, using interpolate_material_output_into_field(const unsigned int c)
.
Alright, I made some comments in the code based on what I think might be the problem, but the test file visco_plastic_copy_and_diffuse.prm still crashes for me (although it solves for the seed field now). So let me know if that helped, and if you encounter any other problems. |
Thank you @jdannberg. I will have to work more on the test but this is a huge help! |
@jdannberg : After running some tests from simple setups, it appears to me that without diffusion and just |
@alarshi, I'll have a look at this tomorrow. |
@alarshi, can you explain a bit more which parameter files exactly you ran that gave you different results? One reason I can imagine why the results would still be different, even if the diffusivity is set to zero, is that a field that evolves using the Or is that not the problem you are seeing? |
@jdannberg Thank you, this is the exact problem I was trying to understand. I used composition-copy-and-diffuse.prm parameter file, I still need to make sense of the values of the density in the results but it is much clearer now. |
@jdannberg : I am still confused in the results. If we run the current parameter file with |
I think the problem is that the model setup is a little confusing, as the model density depends on the compositional field that you use to track the density, so you would not set up a real model like this. Essentially what happens is that the density is copied into a compositional field, then it is diffused, and then the actual density in the model is computed as the reference density + the Density differential * c. new density = reference density + Density differential * old_density So in our example, the background density starts out as being 1 (before time step 0), which is the reference density. Assuming a Density differential = 0.4, that gives us a new density = 1 + 0.4 * 1 = 1.4 after time step 0, and a new density = 1 + 0.4 * 1.4 = 1.56 after time step 1 and so on. So the background density is totally independent of the diffusivity. In a real model, you wouldn't set up the compositional field in this way if you wanted to use it to track the density. You would start out initializing the compositional field with the real density, and then having a reference density of 0 and a Density differential of 1 for the field that tracks the density (so essentially, you would use your new compositional field as the density). Just to demonstrate how that would look like, I've modified the |
Ah, I understand now, thank you so much! |
@alarshi |
Thank you so much @jdannberg for offering your help. I think we are close but not there yet. |
Yes, you can use the outputs copied into the |
Understood, thanks! :) |
@jdannberg : We realized recently that the equation we were trying to solve for the unknown, D, is - |
So do you still have D anywhere in the equation (like in the source term), or only \dot{D}? If you only have \dot{D}, I think it would be quite easy to still use the diffusion solver, you just have to change some terms (and even if you have D itself in the source term, I think this approach might still work). What we are doing at the moment is something like D_new/Delta_t - \nabla \kappa \nabla D_new = D_old/Delta_t + source (where Delta_t would be the time step, and in the assembly we multiply by Delta_t). D_new - \nabla \kappa \nabla D_new = source (so in the assembly in advection.cc, you would have to remove the term with scratch.old_field_values[q] in line 393, and you would have to remove the timestep variable). So if I change the terms that are in advection.cc in your pull request, I think it should look like
and
Depending on the source term (I don't quite remember what has to go in there), the right-hand sight might look slightly different. If you don't need the reaction_term, you might as well remove that, otherwise you would have to make sure the equation is never solved for a zero time step size. I think that the solver should still be able to deal with this type of equation. That means that your equation is basically instantaneous now (the time step doesn't go in there anymore) and you are solving for \dot{D} instead of D. So what you then have in your compositional field is \dot{D}, and if you need the real D anywhere and not just its update, you would have to compute it as D_new = D_old + \dot{D} * timestep. Does that sound like it could work for what you want to do? So I guess that means that you need to solve a different equation compared to what we were planning to do (we really want to solve the original diffusion equation). So I would say, your first priority is probably getting this to work for the problem that you want to solve, and once this is working, we can think about how to finish the pull request. However, it might be easier for me to help if you push your updates to the branch (no need to clean them up beforehand). In the meantime, I might make a separate pull request with the equation that we want to solve, but using the same general structure of the code (having a new advection method, a new assembler, and using the copy fields) so that once you finish up this pull request it should fit into the general structure, and would just add a new method for solving the diffusion problem. This would also mean that hopefully you won't get too many conflicts when you rebase. |
@jdannberg : Thank you very much!, this would work! :D . This PR is up-to date with my local branch. I will get this working and push the commit once done. Thank you again. |
@jdannberg: Hi Juliane. Since we have a working model of diffusion compatible with the recent ASPECT, should I close this PR? |
This branch has implementation of the diffusion scheme written by Julianne in a simple material model. A test is included for simple model but more changes for diffused strain weakening is yet to be done.