Merging Casey's Thesis Work

[csdechant]

This PR directly addresses and should close Issue #257. This PR is based on @cticenhour thesis-rebase branch, excluding $H_{\phi}$ objects. The main focus of this PR includes:

• The includes of a new material object, FieldSolverMaterial, which allows uses to supply the electric field terms in the drift-diffusion equation as a electrostatic potential variable or a vector electric field variable.
• A material object to calculate the plasma dielectric coefficient.

This PR also includes updates based on @csdechant ZapdosAD-plus (which was rebased early on into the thesis-rebase branch), the excluding FV objects. That work includes:

• Kernel objects for the electron energy flux using a thermal conductivity and an effected electric field calculation for ions.
• Boundary conditions objects using an effected electric field calculation for ions and a dielectric time integral boundary condition.
• Tests based on the method of manufactured solutions (MMS).
• Minor bug fixes to the Newton-Shooting method acceleration objects.

[csdechant]

@cticenhour this PR has a lot of commit to begin with. I am in favor of squashing all the commit into one, but since this PR is based on your old branch, I wanted to get your opinion first.

[csdechant]

@cticenhour I am fixing the Precheck errors and one of them is a "banned keywords" error. The PlasmaDielectricConstant material object uses std::cout. I am suggest that I just comment out the section of code and leave a NOTE: comment stating that users can uncomment this section of code for debugging purposes. What are your thoughts?

[moosebuild]

Job Precheck, step Clang format on 7014a11 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/zapdos/docs/PRs/265/clang_format/style.patch | git apply -v

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

git clang-format 402e17523a5ef51b0dac40c3c8175cd976b6ce48

[moosebuild]

Job Documentation, step Sync to remote on 7b1f333 wanted to post the following:

View the site here

This comment will be updated on new commits.

[csdechant]

@cticenhour @gsgall This PR is ready for review. The same as for the other active PR, #263, @gsgall is planned to be the primary reviewer (since this PR involved Casey's and my work). There are two issues I want to point out:

• Depending on which PR is merged first (this one or #263), the other PR will most likely need to be rebased due to replacing _grad_potential with _electric_field.
• There is a test fail due to a Exodiff for tutorial/tutorial05-PlasmaWaterInterface. In particular, the difference is with the Current_OHm and EFieldx1 auxvariables with a relative difference of 3.46855e-05 and 1.67417e-05, respectively. This error only occurs with CIVET and I have not been able to replicated with my Mac (Intel processor). I am in favor of increasing the ceiling threshold for these variables, but I wanted you guys input first.

[csdechant]

• There is a test fail due to a Exodiff for tutorial/tutorial05-PlasmaWaterInterface. In particular, the difference is with the Current_OHm and EFieldx1 auxvariables with a relative difference of 3.46855e-05 and 1.67417e-05, respectively. This error only occurs with CIVET and I have not been able to replicated with my Mac (Intel processor). I am in favor of increasing the ceiling threshold for these variables, but I wanted you guys input first.

I was able to replicate some of this test error with my Ubuntu machine (only a difference for Current_OHm showed up). The current ceiling for the auxvariable for this test is 9.9e-6. I would recommend to increase it to 4e-5 just for auxvariables.

[gsgall]

@cticenhour @gsgall This PR is ready for review. The same as for the other active PR, #263, @gsgall is planned to be the primary reviewer (since this PR involved Casey's and my work). There are two issues I want to point out:

• Depending on which PR is merged first (this one or Updating Doc String: Address Issue #258 #263), the other PR will most likely need to be rebased due to replacing _grad_potential with _electric_field.
• There is a test fail due to a Exodiff for tutorial/tutorial05-PlasmaWaterInterface. In particular, the difference is with the Current_OHm and EFieldx1 auxvariables with a relative difference of 3.46855e-05 and 1.67417e-05, respectively. This error only occurs with CIVET and I have not been able to replicated with my Mac (Intel processor). I am in favor of increasing the ceiling threshold for these variables, but I wanted you guys input first.

I will have time to work on the reviews for both of these later in the week. I still have a last homework assignment for the semester I am working on finishing.

[gsgall]

• There is a test fail due to a Exodiff for tutorial/tutorial05-PlasmaWaterInterface. In particular, the difference is with the Current_OHm and EFieldx1 auxvariables with a relative difference of 3.46855e-05 and 1.67417e-05, respectively. This error only occurs with CIVET and I have not been able to replicated with my Mac (Intel processor). I am in favor of increasing the ceiling threshold for these variables, but I wanted you guys input first.

I was able to replicate some of this test error with my Ubuntu machine (only a difference for Current_OHm showed up). The current ceiling for the auxvariable for this test is 9.9e-6. I would recommend to increase it to 4e-5 just for auxvariables.

That seems fine to me, but It seems like we have to keep raising tolerances on testing. Maybe we should re think how we do testing and move to more robust tests in the future.

[csdechant]

That seems fine to me, but It seems like we have to keep raising tolerances on testing. Maybe we should re think how we do testing and move to more robust tests in the future.

That is true and Issue #261 should help with that (currently working on that issue). For more reference with this test (tutorial/tutorial05-PlasmaWaterInterface), it is basically a tutorial version of a current test 1d_dc/mean_en. Looking at the cmp files, it seems that 1d_dc/mean_en uses looser tolerances for the auxvariables that failed in tutorial/tutorial05-PlasmaWaterInterface (they are set to 9.6e-5 for mean_en_out.cmp). I will set the new tolerances based on mean_en_out.cmp and in the future we should try on consolidated these tests.

[moosebuild]

Job Precheck, step Size check on 7b1f333 wanted to post the following:

Warning: This PR changes repo size by 15.72 MiB.

[cticenhour]

@gsgall My two cents - OK with loosening tolerances here in this case (at least to the point we are consistent with a similar test elsewhere in the suite, as @csdechant pointed out), but I am hoping that starting work on re-vamping our V&V tests in #261 will make our approach to testing more granular and more targeted overall. We can't troubleshoot nearly as quickly when most of our tests are large and purely physics-application-driven.

Given the number of platforms we're developing and testing against, I expect some custom CMP files or adjusted tolerances here and there, but we should try not to make loosening tolerances the norm - I agree!

[cticenhour]

I don't want to get in the way of @gsgall's review, but I have one concern about the size of the PR. 15MB is way too big. @csdechant - please look into how we can reduce the size of the new gold files by limiting the output we're diffing to only certain times in the simulation.

[gsgall]

I don't want to get in the way of @gsgall's review, but I have one concern about the size of the PR. 15MB is way too big. @csdechant - please look into how we can reduce the size of the new gold files by limiting the output we're diffing to only certain times in the simulation.

I definitely think keeping the gold files size at a minimum should be a priority. If we are adding more capability with this PR then I think a lot of the testing should be really simple V&V testing. Which in my experience only requires small gold files.

@csdechant Is the testing strategy of this PR similar to the larger physics based testing used for the majority Zapdos testing? Or is it more math based, focusing on the simplest tests possible for each kernel, i.e having a single kernel in an input whenever possible.

[csdechant]

I don't want to get in the way of @gsgall's review, but I have one concern about the size of the PR. 15MB is way too big. @csdechant - please look into how we can reduce the size of the new gold files by limiting the output we're diffing to only certain times in the simulation.

@cticenhour This is a very fair request and I will re-work the gold files. Currently the new tests looks at all time steps, but I will edit it to just look at every $\frac{1}{4}T$ time steps ($T$ being the period of the cycle).

[csdechant]

@csdechant Is the testing strategy of this PR similar to the larger physics based testing used for the majority Zapdos testing? Or is it more math based, focusing on the simplest tests possible for each kernel, i.e having a single kernel in an input whenever possible.

@gsgall All new test are math based (MMS to be exact) and in my option, all new tests in Zapdos should be against some type of known solution (either analytical or MMS) and any new test based on validation should be on a case-by-case basis. For the new MMS test, they are designed to test problems of increase coupling (i.e. one for diffusion-only, next with advection-diffusion with a function potential, next with advection-diffusion with variable potential, etc.).

The reason for the large gold files is that these MMS tests use solutions designed for an RF discharge, so the test are for one RF cycle. The current tests looks at every time step, so the gold files are big (in honesty, I could probably increase the step size too, as the original step size was chosen to be significantly small enough not to interfere with spatial convergence studies).