-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add grad_zero for phi and test function #15205
Conversation
_grad_phi_zero[tid].resize(max_qpts, {0., 0., 0.}); | ||
_grad_test_zero[tid].resize(max_qpts, {0., 0., 0.}); |
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.
@lindsayad this init here is ugly but I can't figure out how to init with RealVectorValue...
suggestions?
I need to see a test to believe this is working. For one thing the outer size should be according to the number of shape functions, not the number of q-points. Perhaps you are getting away with this because for first-order Lagrange those numbers are the same in the element volume? Also my current reading of the |
Job Documentation on e63ae8d wanted to post the following: View the site here This comment will be updated on new commits. |
wait, that is not how the already available I'm not testing it right now, try to make a kernel or material for this |
A |
This makes a lot of sense and explains why I was not able to find the right initialization for it.
|
Correct. You might actually want to look at the code around this line: https://github.com/idaholab/moose/blob/next/framework/src/problems/FEProblemBase.C#L627. We actually compute exactly what you're looking for there |
Job Modules debug PETSc alt on 2d2630c : invalidated by @arovinelli uhm... this should be related to this pr |
@lindsay I'm also trying to construct a |
A |
@lindsay it isn't as straightforward as I thought. I tried to use the code you indicated , inside
here is the stack trace
so I can't compute either the maxNdof per elem or node |
It might be helpful for you to look at the order of actions in Why don't you wait to do your zeroing and setup until |
@lindsay, thanks a lot for the suggestion I've embraced it and I'm making good progress. moose/framework/src/problems/FEProblemBase.C Lines 1612 to 1613 in 544d67e
|
238a3de
to
3ea16a8
Compare
Looking at MaxQpsThread::operator(), `getMaxShapeFunctions` will definitely
return the wrong value if there are second order bases.
…On Wed, May 13, 2020 at 1:20 PM Andrea Rovinelli ***@***.***> wrote:
@lindsay <https://github.com/lindsay>, thanks a lot for the suggestion
I've embraced it and I'm making good progress.
At this point, I believe the size of _second_phi_zero wrong and also
initialized in the wrong place, could you check (see link)
https://github.com/idaholab/moose/blob/544d67eb7d02227ed071bd74b677aeb5674c8d24/framework/src/problems/FEProblemBase.C#L1612-L1613
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15205 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACOGA4FDE3WS4NAZICSN6S3RRL6IFANCNFSM4MXNV7GQ>
.
|
Job Test debug on 2a4ef51 : invalidated by @arovinelli these failures should not be related to this PR |
So this moose/framework/src/problems/FEProblemBase.C Lines 1612 to 1613 in 544d67e
is a preexisting bug.
|
2a4ef51
to
75a8f5f
Compare
test/src/kernels/PhiZeroKernel.C
Outdated
} | ||
|
||
PhiZeroKernel::PhiZeroKernel(const InputParameters & parameters) | ||
: NullKernel(parameters), _second_phi(_assembly.secondPhi(_var)) |
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.
@lindsayad Why I can't get _second_phi.size() > 0? In my tests, I use second-order element and second-order lagrangian function
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.
Just calling secondPhi
in Assembly
is not enough to get MOOSE to do second derivatives. You need to actually request something like _var.secondSln()
in order to get it to happen. A hacky way that may also work is to directly call _assembly.feSecondPhi<Real>(FEType(LAGRANGE,FIRST))
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.
thanks, now it works!
Job Create Linux Stack on 667552f : invalidated by @arovinelli stucked |
@lindsayad when you have time, this is ready for review |
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.
A nice addition. thank you @arovinelli . I'm curious what your use case is?
Mostly fixup the test specs file
if (haveADObjects() || (_displaced_problem && _displaced_problem->haveADObjects())) | ||
{ | ||
CONSOLE_TIMED_PRINT("Computing max dofs per elem/node"); | ||
// alwasy esecute to get the max number of DoF per elment and node needed to initialized phi_zero |
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.
always execute
test/include/kernels/PhiZeroKernel.h
Outdated
|
||
#include "NullKernel.h" | ||
|
||
class PhiZeroKernel; |
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.
No need for this forward declaration
test/include/kernels/PhiZeroKernel.h
Outdated
/** | ||
* | ||
*/ |
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.
Either add a doxygen comment or just delete this
test/src/kernels/PhiZeroKernel.C
Outdated
"_phi.size() " + std::to_string(_phi.size()) + "> _phi_zero.size() " + | ||
std::to_string(_phi_zero.size())); |
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 understand that you were thinking about cases in which the "zero" members were of zero size, but your message should probably just say "!=" as opposed to ">". Same for all the following assertions
test/tests/phi_zero/tests
Outdated
input = 'simple_transient_diffusion.i' | ||
expect_out = ' Solve Converged!' | ||
issues = '#15204' | ||
design = ' ' |
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.
ruh roh. You should either write some documentation or find some existing documentation that roughly fits what you're working on
test/tests/phi_zero/tests
Outdated
issues = '#15204' | ||
design = ' ' | ||
requirement = 'The system shall be able to produce consestent size _phi_zero and _grad_phi_zero objects' | ||
method = 'dbg' |
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.
You might as well remove this as well. I understand that your assertions won't be tested, but the problem should still run for all methods
test/tests/phi_zero/tests
Outdated
cli_args = 'Mesh/elem_type=QUAD8 Variables/dummy/order=SECOND Variables/u/order=SECOND Kernels/phi_zero/second_order=true' | ||
requirement = 'The system shall be able to produce consestent size _phi_zero and _grad_phi_zero' | ||
'objects for quadratic shape functions' | ||
expect_out = ' Solve Converged!' |
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.
Again can remove
test/tests/phi_zero/tests
Outdated
'objects for quadratic shape functions' | ||
expect_out = ' Solve Converged!' | ||
issues = '#15204' | ||
design = ' ' |
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.
need doc
test/tests/phi_zero/tests
Outdated
type = 'RunApp' | ||
input = 'simple_transient_diffusion.i' | ||
cli_args = 'Mesh/elem_type=QUAD8 Variables/dummy/order=SECOND Variables/u/order=SECOND Kernels/phi_zero/second_order=true' | ||
requirement = 'The system shall be able to produce consestent size _phi_zero and _grad_phi_zero' |
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.
could use a similar requirement to the one I proposed above
test/tests/phi_zero/tests
Outdated
expect_out = ' Solve Converged!' | ||
issues = '#15204' | ||
design = ' ' | ||
method = 'dbg' |
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.
remove
c8047b6
to
df9389f
Compare
@lindsay This is a preparation step for implementing large deformation cohesive zone modeling. The main reason I needed this is to ease the formulation of the jacobian of the traction w.r.t. the discreet displacements. It might not be optimal in terms of efficiency but make the code much more understandable. It will also allow using exactly the same formulation for 1D 2D and 3D problems, thus avoiding a bunch of if statements that are detrimental when performing maintenance.
The last PR should address all your comments. Check the design and requirement parts to see if they comply with your specs. Alex, thanks again for helping with this!! |
df9389f
to
20c49a2
Compare
Job Private App tests PETSc alt on bd98388 : invalidated by @lindsayad |
Job Documentation on bd98388 : invalidated by @arovinelli something is funny, try rerun |
bd98388
to
1a8724a
Compare
I can't put the |
1a8724a
to
3e28837
Compare
@lindsay this has passed all tests, please give it another review and let me know if there is something left to do. |
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 really like how this looks. Just the two tiny doc requests, and then I'll approve!
# PhiZero | ||
|
||
MOOSE has helper zero objects for shape functions and shape function gradients that can be accessed by all MOOSE objects. The names of the zero objects are: `_phi_zero`, `_grad_phi_zero` and `_second_phi_zero`. | ||
These object first two dimensions are,: (i) the maximum number of degree of freedom per variable per element, and (ii) the maximum number of quadrature points per element. |
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.
remove comma in are,
|
||
MOOSE has helper zero objects for shape functions and shape function gradients that can be accessed by all MOOSE objects. The names of the zero objects are: `_phi_zero`, `_grad_phi_zero` and `_second_phi_zero`. | ||
These object first two dimensions are,: (i) the maximum number of degree of freedom per variable per element, and (ii) the maximum number of quadrature points per element. | ||
The `PhiZeroKernel` checks that the first two dimensions of `_phi_zero`, `_grad_phi_zero` and `_second_phi_zero` are |
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.
Probably say the "PhiZeroKernel
test object"
Just add two constants gradient variable
@lindsayad as per mailing list discussion
ref #15204