Website Update - AuxKernels

[csdechant]

This will be the start of a series of PRs to update the documentation of the Zapdos website. This PR includes:

• Updated documentation to the Zapdos AuxKernel System.
• Edits in flux AuxKernels to allow for the output of any component of the flux.
• Minor edits in nomenclature for three body reactions to better a line with CRANE terminology.

[moosebuild]

Job Documentation on 5cb196a wanted to post the following:

View the site here

This comment will be updated on new commits.

[csdechant]

Due to the discussion from Issue #223, I am breaking up the website documentation update PR by system.

I am formally letting @gsgall review this PR, but I am mentioning @cticenhour and @lindsayad, in case they have some input on the documentation.

[gsgall]

I like the fact that we are generalizing the vector components here but I think my preference would be for these AuxKernels to output VectorValues rather than scalars so we can reduce the length of an input file and possibly reduce error in the case that someone may forget a component. Is there a specific reason you chose to allow the user to request a specific component rather than calculating all 3 and storing it in a vector?

[csdechant]

Is there a specific reason you chose to allow the user to request a specific component rather than calculating all 3 and storing it in a vector?

The simple reason is because the AuxKernels were originally made and tested with scalar variables. Converting these AuxKernels to output vectors would require to re-gold several tests, which I would like to avoid for a PR that focuses on documentation and only include minor bug fixes, when needed. If we want to pursue changing these scalar AuxKernels into vector AuxKernels, I believe it should be a separate PR.

[lindsayad]

Thanks for the ping @csdechant but I think I'll let you guys handle it!

[cticenhour]

@csdechant Thanks for the ping, and very happy to let @gsgall give the formal review. Since he doesn't currently have write access, I propose we use this PR as one of several to let Grayson get his bearings on normal MOOSE App PR reviews, with the eventual goal of adding him to the Zapdos Change Control Board (i.e., those with write privileges on the repo). I can give the formal approval to make GitHub happy and comment on the review before we merge. Does this sound OK with you?

[csdechant]

@cticenhour That sounds good to me.

[gsgall]

Is there a specific reason you chose to allow the user to request a specific component rather than calculating all 3 and storing it in a vector?

The simple reason is because the AuxKernels were originally made and tested with scalar variables. Converting these AuxKernels to output vectors would require to re-gold several tests, which I would like to avoid for a PR that focuses on documentation and only include minor bug fixes, when needed. If we want to pursue changing these scalar AuxKernels into vector AuxKernels, I believe it should be a separate PR.

That makes sense to me. This may be something to pursue but we can open a different issue for this if we want to do this. What are your thoughts on making this a new issue @cticenhour @lindsayad @keniley1

[lindsayad]

What are your thoughts on making this a new issue @cticenhour @lindsayad @keniley1

sounds good

[cticenhour]

I agree

[gsgall]

Overall it looks good most of the suggestions I've made are small formatting and grammatical changes. While it was not submitted in these changes I think we should take this opportunity to fix some other grammatical errors in the class descriptions. The ones I would like to see corrected all share the phrase "of defined species" and these should be more like "of the specified species"

• Current
• DiffusiveFlux
• EFieldAdvFlux
• TotalFlux
• DriftDiffusionFluxAux
• AbsValueAux (not the same error but should be something like "value of the specified variable"

[moosebuild]

Job Precheck on 7bf44ba 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/225/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 f67c46f29b7667369898ffcf9dc2e302db450f50

[csdechant]

Done addressing all of revision suggestions. I overlooked adding all the suggests as one batch, so there are quite a few commits added to this PR. I am planning to squash all these commit to a single commit label "Grammatical Revision", then add a last commit label "Adding Test Warnings".

@cticenhour @gsgall are either of you against squashing all of these new commits (4fc8676 - 323da7d)?

[cticenhour]

I would squash them!

[csdechant]

@gsgall everything should be ready to go to finish this PR. All that should be left is to review the additions of the undocumented test warnings and add edits, if needed.

[gsgall]

Great job on the corrections @csdechant . I have a few more small formatting corrections that I must have missed the first time round. The only change I could not suggest, the source file had no changes in this commit, was the appearance of the #/m^3 in DensityMols but it just needs the math environment on some of it.

The other suggestion I made was the warning used for the untested classes. You had put Undocumented Test and I am assuming that may be because at one point these classes were tested before they were made a part of Zapdos and if that is the case or you have another reason I am fine with that. But I also made the suggestion for Untested Class for the warnings.

[csdechant]

@gsgall everything should be ready to go.

[gsgall]

Great job Corey, looks good to me!

[cticenhour]

Stepping in to do the formal review now that @gsgall has given his approval. Good job overall Grayson!

@csdechant - thanks for your work here! I found a few things I wanted to point out, but all of this is relatively minor. Should be good to do once these are addressed.