Time Derivative for Array Variables #14059

zachmprince · 2019-09-18T18:25:23Z

Adding ArrayTimeKernel for base class of array kernels involving time derivatives
Adding ArrayTimeDerivative for computing time derivative of array variables
Adding transient test and documentation

rwcarlsen · 2019-09-18T18:44:15Z

framework/include/kernels/ArrayTimeDerivative.h

+
+protected:
+  virtual RealEigenVector computeQpResidual() override;
+  virtual RealEigenVector computeQpJacobian() override;


Should these maybe be ArrayVariableValue (which evaluates to the same type)?

That's possible, but the base class (ArrayKernel) has these defined as RealEigenVector. Would it be inconsistent to change it in this kernel?

rwcarlsen · 2019-09-18T18:53:32Z

framework/src/kernels/ArrayTimeDerivative.C

+{
+  if (_coeff_type == 0)
+    _coeff = &getMaterialProperty<Real>("time_derivative_coefficient");
+  else if (_coeff_type == 1)


We could move these checks into the initialSetup function and then use hasMaterialProperty<Real/RealEigenVector/RealEigenMatrix>("time_derivative_coeff") to determine the type of the material. Then users don't have to set/remember anything about the coefficient type and it just works automatically.

Good idea! I'll switch to using hasMaterialProperty instead of asking for it from the user.

Note: I get an error when I move these checks to initialSetup.
*** ERROR ***
Material properties must be retrieved during object construction. This is a code problem.

Okay - this is going to have to go back into the constructor due to quirks of object initialization order. The constructor only works in this case because the object you are coding is not a material.

I kept the getMaterialPropertys in the constructor but I had to move the mooseAsserts into computeQpResidual because of an error I was getting in debug mode. It seems I couldn't access the size of _coeff until then.

That is fine/expected.

moosebuild · 2019-09-18T19:33:39Z

Job Documentation on f7fcbd8 wanted to post the following:

View the site here

This comment will be updated on new commits.

permcody · 2019-09-18T23:01:23Z

@zachmprince - I sent you an invite so your PRs will start testing. Please accept.

zachmprince · 2019-09-18T23:09:47Z

@permcody - I don't see the invite you sent, but it seems the PR is automatically testing.

permcody · 2019-09-18T23:13:52Z

@permcody - I don't see the invite you sent, but it seems the PR is automatically testing.

Nope - I'm just on top of CIVET. Everyone always says this too me. It should go to whatever your email address is listed as in Github.com. Might be a personal email?

zachmprince · 2019-09-18T23:22:05Z

Nope - I'm just on top of CIVET. Everyone always says this too me. It should go to whatever your email address is listed as in Github.com. Might be a personal email?

Oh found it, sorry about that.

YaqiWang · 2019-09-19T03:25:19Z

Zach, did you compare the array version of input with the non-array version with the exactly same solver settings? We should be able to get the exactly same convergence. Because you removed the coefficient type parameter, it could be good to do the same for the diffusion and reaction kernels.

zachmprince · 2019-09-19T14:41:47Z

Zach, did you compare the array version of input with the non-array version with the exactly same solver settings? We should be able to get the exactly same convergence. Because you removed the coefficient type parameter, it could be good to do the same for the diffusion and reaction kernels.

@YaqiWang The convergence is very similar but not identical. As in, the number of linear and nonlinear iterations are exactly the same for every time step, but the residuals don't quite match. They are off by ~0.5% throughout the transient. Not sure why this would be. I've attached the console outputs of the cases I ran.
array_diag_pjfnk.txt
array_jfnk.txt
array_smp_newton.txt
array_smp_pjfnk.txt
diag_pjfnk.txt
jfnk.txt
smp_newton.txt
smp_pjfnk.txt

rwcarlsen · 2019-09-19T14:58:22Z

framework/src/kernels/ArrayTimeDerivative.C

+RealEigenVector
+ArrayTimeDerivative::computeQpResidual()
+{
+  if (_coeff_type == 0)


These could probably just check if (_coeff/_coeff_array/_coeff_2d_array) since only one will be non-null. Then we don't need _coeff_type at all.

I removed _coeff_type per your suggestion. But I kept getting random crashes, I guess because the null ones were never initiated. So I switched to a ternary operator to assure the unused ones are initiated to nullptr.

Yes, as a general rule, we need to make sure every member variable that is a POD type (e.g. pointers, ints, etc.) is explicitly initialized in the constructor. What you did is fine. Another fine option would be to just set all of them to nullptr in the initialization list and leave the if checking code in the constructor body like before.

YaqiWang · 2019-09-19T15:21:38Z

I looked array_smp_newton.txt and smp_newton.txt. I think they match well. Small differences in residual norms could be just expected due to the numerical round-off.

moosebuild · 2019-09-19T16:04:09Z

Job Precheck on 513996f wanted to post the following:

Your code requires style changes.

A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:

curl -s https://mooseframework.inl.gov/docs/PRs/14059/style.patch | git apply -v

Alternatively, with your repository up to date and in the top level of your repository:

git clang-format ba456f4db1ca36ff1a96cdb4f7fd5402a1a7a77b

…daholab#14057

…tion idaholab#14057

YaqiWang · 2019-09-19T16:32:36Z

Thanks @rwcarlsen for the idea and @zachmprince for the cleanup of array kernels I did before ;-)

rwcarlsen · 2019-09-19T19:23:18Z

The failures are caused by network issues on the INL side. Not this PR.

rwcarlsen suggested changes Sep 18, 2019

View reviewed changes

zachmprince force-pushed the array_time_14057 branch 2 times, most recently from 2b2ab24 to bdb18ac Compare September 18, 2019 22:59

rwcarlsen reviewed Sep 19, 2019

View reviewed changes

zachmprince force-pushed the array_time_14057 branch from bdb18ac to 513996f Compare September 19, 2019 16:02

zachmprince added 2 commits September 19, 2019 10:04

Adding array kernel for computing time derivates of array variables i…

fb71ebe

…daholab#14057

Removing coefficient_type parameter from ArrayDiffusion and ArrayReac…

f7fcbd8

…tion idaholab#14057

zachmprince force-pushed the array_time_14057 branch from 513996f to f7fcbd8 Compare September 19, 2019 16:09

rwcarlsen approved these changes Sep 19, 2019

View reviewed changes

permcody merged commit 1575de5 into idaholab:next Sep 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Time Derivative for Array Variables #14059

Time Derivative for Array Variables #14059

zachmprince commented Sep 18, 2019

rwcarlsen Sep 18, 2019

zachmprince Sep 18, 2019

rwcarlsen Sep 18, 2019

zachmprince Sep 18, 2019

zachmprince Sep 18, 2019

rwcarlsen Sep 18, 2019

zachmprince Sep 18, 2019

rwcarlsen Sep 19, 2019

moosebuild commented Sep 18, 2019 •

edited

permcody commented Sep 18, 2019

zachmprince commented Sep 18, 2019

permcody commented Sep 18, 2019

zachmprince commented Sep 18, 2019

YaqiWang commented Sep 19, 2019

zachmprince commented Sep 19, 2019

rwcarlsen Sep 19, 2019

zachmprince Sep 19, 2019

rwcarlsen Sep 19, 2019 •

edited

YaqiWang commented Sep 19, 2019

moosebuild commented Sep 19, 2019

YaqiWang commented Sep 19, 2019

rwcarlsen commented Sep 19, 2019

Time Derivative for Array Variables #14059

Time Derivative for Array Variables #14059

Conversation

zachmprince commented Sep 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moosebuild commented Sep 18, 2019 • edited

permcody commented Sep 18, 2019

zachmprince commented Sep 18, 2019

permcody commented Sep 18, 2019

zachmprince commented Sep 18, 2019

YaqiWang commented Sep 19, 2019

zachmprince commented Sep 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwcarlsen Sep 19, 2019 • edited

Choose a reason for hiding this comment

YaqiWang commented Sep 19, 2019

moosebuild commented Sep 19, 2019

YaqiWang commented Sep 19, 2019

rwcarlsen commented Sep 19, 2019

moosebuild commented Sep 18, 2019 •

edited

rwcarlsen Sep 19, 2019 •

edited