Implementation of plastic potential for VP and EVP.#649
Conversation
|
Readthedocs is failing. Looks like "TC" is not defined in the new Ringler reference. |
|
It should work now. Thanks! |
eclare108213
left a comment
eclare108213
left a comment
There was a problem hiding this comment.
The documentation changes were made only in the VP section. Is this new yield curve only available for VP, or could it be used for the other dynamics options too?
|
The VP section is an introduction for the implicit and EVP approaches. It is in this section that we introduce the viscous coefficients and Delta. The new plastic potential affects the definition of eta (viscous coeff) and Delta. It is therefore available for the implicit solver (VP), EVP and rEVP. I think the first sentence in the EVP section suggests this: "In the EVP model the internal stress tensor is determined from a regularized version of the VP constitutive law" Do we need more info? |
|
It wasn't obvious to me, reading all of those sections, that those options were available for all of the (E)VP variants. Please make that explicit. When we rework the dynamics drivers etc we should take a look at the documentation too (or maybe vice-versa, as guidance). |
phil-blain
left a comment
phil-blain
left a comment
There was a problem hiding this comment.
Thanks @JFLemieux73. It's nice how this can be implemented with so little changes to the code. I left a few comments below.
| \sigma_{ij} = 2 \eta \dot{\epsilon}_{ij} + (\zeta - \eta) D_D - P_R(1 - k_t)\frac{\delta_{ij}}{2} | ||
| \sigma_{ij} = 2 \eta \dot{\epsilon}_{ij} + (\zeta - \eta) D_D - P_R\frac{\delta_{ij}}{2} |
There was a problem hiding this comment.
Here and below we remove 1 - k_t from the equation, I guess because we have not introduced the tensile strength yet (with the changes you do in this PR). So I guess this is OK.
There was a problem hiding this comment.
The replacement pressure now includes the (1-k_t) term. This is consistent with Ringeisen et al. 2021.
There was a problem hiding this comment.
I'm a little confused about this too. I think I understand what's happening in the code. What's confusing is that P and replacement pressure aren't quite the same, and in our docs both are referenced to old publications, which don't include the new tensile strength. So when we clean up the dynamics documentation as part of the overall C-grid effort, let's fix that too.
| The tensile strength :math:`T_p` is expressed as a fraction of :math:`P`, that is :math:`T_p=k_t P` | ||
| where :math:`k_t` should be set to a value between 0 and 1 (this can | ||
| be changed at runtime with the namelist parameter ``Ktens``). |
There was a problem hiding this comment.
It would be clearer if T_p appeared in the equations below, or at least mention how its effect is incorporated in the viscous coefficients (since k_t is removed from the equations).
There was a problem hiding this comment.
I have removed T_p in the doc.
| !ecci = c1/ecc ! 1/ecc | ||
| ecci = c1/e_ratio**2 ! 1/ecc | ||
| ! variables for elliptical yield curve and plastic potential | ||
| epp2i = c1/e_plasticpot**2 |
There was a problem hiding this comment.
not sure I like that name.. but I don't have anything much better to propose :P
There was a problem hiding this comment.
At least it looks like the previous variable ecci...
|
@eclare108213 I agree we should have a look and probably modify some of the doc for the dynamics. Note that I have reordered some subsections (EAP is now at the end as it is really different than the other ones). I also think we should rework the revised EVP subsection. Some eqs should go in the momentum section and not in the internal stress section. |
|
@eclare108213 I think we should go ahead with this PR. I plan to go back to the documentation soon. I think we can make it simpler. For example we have two sections EVP and two sections VP...these can be combined. |
eclare108213
left a comment
eclare108213
left a comment
There was a problem hiding this comment.
I'm willing to approve and merge these changes, under the assumption that the documentation will be fixed fairly soon.
| \sigma_{ij} = 2 \eta \dot{\epsilon}_{ij} + (\zeta - \eta) D_D - P_R(1 - k_t)\frac{\delta_{ij}}{2} | ||
| \sigma_{ij} = 2 \eta \dot{\epsilon}_{ij} + (\zeta - \eta) D_D - P_R\frac{\delta_{ij}}{2} |
There was a problem hiding this comment.
I'm a little confused about this too. I think I understand what's happening in the code. What's confusing is that P and replacement pressure aren't quite the same, and in our docs both are referenced to old publications, which don't include the new tensile strength. So when we clean up the dynamics documentation as part of the overall C-grid effort, let's fix that too.
|
One small point, maybe we want to add a new test with a non-default value of |
|
I added a note in issue #651 for the new test. I will do that later. Can we please merge this in the main code? I would like to have the plastic potential for next week for the C-grid implementation of rheology. |
|
@JFLemieux73 I should have noticed that sooner, but maybe it would have been a good idea to keep I just had a failure with the new code and a namelist from a few weeks ago, that's why I noticed... |
|
I think this complicates things. With time backward compatibility is less and less important so I would not worry about this. Moreover I find that it is a good idea to get rid completely of e_ratio. e_yieldcurve is a better name. |
4 participants
For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers
PR checklist
Implementation of plastic potential approach of Ringeisen et al. 2021
@JFLemieux73
@phil-blain
337 measured results of 337 total results
337 of 337 tests PASSED
0 of 337 tests PENDING
0 of 337 tests MISSING data
0 of 337 tests FAILED
@TillRasmussen FYI: I have not modified the 1d-evp. This is why I kept ecci=1/e_yieldcurve**2.