Allowing to define an array varible with component size one #19567

yjung-anl · 2021-12-07T05:09:07Z

closes #19564

loganharbour

Add a test. Array diffusion with a variable with one component is sufficient.
is_av/av -> array. FV is well known. AV is not

framework/src/variables/MooseVariableBase.C

yjung-anl · 2021-12-07T05:32:08Z

@loganharbour Thank you for super quick review!

loganharbour · 2021-12-07T05:45:20Z

@loganharbour Thank you for super quick review!

Timed out well with watching tv 😆

I’m good with all of this and understand the purpose - we just need to be stringent given that these chunks of code are ran in almost every use of moose

moosebuild · 2021-12-07T06:10:22Z

Job Documentation on 49b4b02 wanted to post the following:

View the site here

This comment will be updated on new commits.

moosebuild · 2021-12-07T07:47:49Z

Job Coverage on 49b4b02 wanted to post the following:

Framework coverage

	c25336	#19567 49b4b0
	Total	Total	+/-	New
Rate	82.17%	82.17%	-0.00%	86.67%
Hits	72864	72872	+8	13
Misses	15806	15809	+3	2

Diff coverage report

Full coverage report

Modules coverage

Chemical reactions

	c25336	#19567 49b4b0
	Total	Total	+/-	New
Rate	93.27%	93.27%	-	100.00%
Hits	873	873	-	2
Misses	63	63	-	0

Diff coverage report

Full coverage report

Contact

	c25336	#19567 49b4b0
	Total	Total	+/-	New
Rate	87.90%	87.90%	-	100.00%
Hits	2273	2273	-	1
Misses	313	313	-	0

Diff coverage report

Full coverage report

Heat conduction

	c25336	#19567 49b4b0
	Total	Total	+/-	New
Rate	88.77%	88.77%	-	100.00%
Hits	2997	2997	-	1
Misses	379	379	-	0

Diff coverage report

Full coverage report

Navier stokes

	c25336	#19567 49b4b0
	Total	Total	+/-	New
Rate	83.16%	83.16%	-	100.00%
Hits	7607	7607	-	3
Misses	1540	1540	-	0

Diff coverage report

Full coverage report

Phase field

	c25336	#19567 49b4b0
	Total	Total	+/-	New
Rate	90.20%	90.20%	-	100.00%
Hits	11926	11926	-	5
Misses	1296	1296	-	0

Diff coverage report

Full coverage report

Richards

	c25336	#19567 49b4b0
	Total	Total	+/-	New
Rate	97.64%	97.64%	-	100.00%
Hits	3346	3346	-	1
Misses	81	81	-	0

Diff coverage report

Full coverage report

Full coverage reports

Reports

Warnings

framework new line coverage rate 86.67% is less than the suggested 90.0%

This comment will be updated on new commits.

yjung-anl · 2021-12-07T16:43:08Z

Changes

av and is_av are renamed to array_variable and is_array_variable. A variable can be explicitly defined as an array variable in input deck using the parameter array_variable=true as:

[Variables]
  [u]
    order = THIRD
    family = MONOMIAL
    block = 0
    components = 1
    array_variable = true
  []
[]

A test case corresponding to this PR was included in test/tests/variables/array_variable/array_variable_size_one_test.i
Rebased on top of next branch

loganharbour

array_variable -> array
Clean up determineType (remove redundant parameter)
Move error check to a better location and add a test for it

I'm good after this!

framework/include/actions/AddVariableAction.h

framework/src/actions/AddVariableAction.C

framework/src/systems/SystemBase.C

framework/src/variables/MooseVariableBase.C

test/tests/variables/array_variable/tests

test/tests/variables/array_variable/array_variable_size_one_test.i

moosebuild · 2021-12-07T21:03:19Z

Job Precheck on 0c178c0 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/19567/style.patch | git apply -v

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

git clang-format d3a9b367b41eb0c9d4b8ecdcafaccbad43bf8d96

…holab#19564

loganharbour · 2021-12-07T22:22:45Z

@lindsayad can you take a look at this please - I know that you're the one that did most of the variable cleaning and I would like a 👍🏼 or 👎🏼 on the AddVariableAction::determineType() deprecation

loganharbour · 2021-12-07T22:27:45Z

I think that we will also want to add

params.set<bool>("array") = true;

in ArrayMooseVariable::validParams()

but I'd like a ✅ from @lindsayad on that one too

lindsayad

I would kind of like to see a coverage report, but this looks fine to me

refs idaholab#19564

loganharbour · 2021-12-07T23:30:25Z

@lindsayad good for another review - I thought that we might of broke something, but we didn't. Decided to keep the test that I made anyway

This basically tests creating an array variable from a MooseArrayVariable instead of a MooseVariableBase

lindsayad

👍 pending a favorable coverage report

loganharbour

Coverage is favorable.

Docs failure is unrelated and patch is currently in bison devel

YaqiWang · 2021-12-08T07:41:07Z

@yjung-anl can you update Griffin really quick to use the new function?

loganharbour requested changes Dec 7, 2021

View reviewed changes

framework/src/variables/MooseVariableBase.C Outdated Show resolved Hide resolved

moosebuild added the PR: Failed but allowed label Dec 7, 2021

yjung-anl force-pushed the array_variable_size_one_fix branch from 92b1697 to 1182e21 Compare December 7, 2021 15:32

moosebuild removed the PR: Failed but allowed label Dec 7, 2021

yjung-anl force-pushed the array_variable_size_one_fix branch from 1182e21 to a91f78e Compare December 7, 2021 15:33

loganharbour requested changes Dec 7, 2021

View reviewed changes

test/tests/variables/array_variable/tests Outdated Show resolved Hide resolved

test/tests/variables/array_variable/array_variable_size_one_test.i Outdated Show resolved Hide resolved

test/tests/variables/array_variable/array_variable_size_one_test.i Outdated Show resolved Hide resolved

moosebuild added the PR: Failed but allowed label Dec 7, 2021

yjung-anl force-pushed the array_variable_size_one_fix branch from a91f78e to 0c178c0 Compare December 7, 2021 21:00

yjung-anl requested review from bwspenc, dschwen, GiudGiud and lindsayad as code owners December 7, 2021 21:00

moosebuild removed the PR: Failed but allowed label Dec 7, 2021

yjung-anl force-pushed the array_variable_size_one_fix branch from 0c178c0 to f4bc80c Compare December 7, 2021 21:06

yjung-anl added 2 commits December 7, 2021 16:10

Allowing to define an array varible with component size one, refs ida…

ef50fd1

…holab#19564

replacing determineType to variableType

e70955b

yjung-anl force-pushed the array_variable_size_one_fix branch from f4bc80c to e70955b Compare December 7, 2021 22:11

lindsayad previously approved these changes Dec 7, 2021

View reviewed changes

loganharbour added 2 commits December 7, 2021 16:27

Change docstring to denote that this is deprecated

e46ee9d

refs idaholab#19564

Use more const

2e5abe1

refs idaholab#19564

loganharbour added 2 commits December 7, 2021 16:28

Add more information on auto setting 'array'

209de50

refs idaholab#19564

Add test for creating array variables in an action

49b4b02

refs idaholab#19564

loganharbour dismissed lindsayad’s stale review via 49b4b02 December 7, 2021 23:29

loganharbour requested a review from lindsayad December 7, 2021 23:29

lindsayad approved these changes Dec 7, 2021

View reviewed changes

loganharbour approved these changes Dec 8, 2021

View reviewed changes

loganharbour merged commit 6463c45 into idaholab:next Dec 8, 2021

YaqiWang mentioned this pull request Mar 12, 2022

Fix checking array variables #20538

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allowing to define an array varible with component size one #19567

Allowing to define an array varible with component size one #19567

yjung-anl commented Dec 7, 2021

loganharbour left a comment

yjung-anl commented Dec 7, 2021

loganharbour commented Dec 7, 2021 •

edited

moosebuild commented Dec 7, 2021 •

edited

moosebuild commented Dec 7, 2021 •

edited

yjung-anl commented Dec 7, 2021

loganharbour left a comment

moosebuild commented Dec 7, 2021

loganharbour commented Dec 7, 2021

loganharbour commented Dec 7, 2021

lindsayad left a comment

loganharbour commented Dec 7, 2021 •

edited

lindsayad left a comment

loganharbour left a comment

YaqiWang commented Dec 8, 2021

Allowing to define an array varible with component size one #19567

Allowing to define an array varible with component size one #19567

Conversation

yjung-anl commented Dec 7, 2021

loganharbour left a comment

Choose a reason for hiding this comment

yjung-anl commented Dec 7, 2021

loganharbour commented Dec 7, 2021 • edited

moosebuild commented Dec 7, 2021 • edited

moosebuild commented Dec 7, 2021 • edited

Framework coverage

Modules coverage

Chemical reactions

Contact

Heat conduction

Navier stokes

Phase field

Richards

Full coverage reports

Warnings

yjung-anl commented Dec 7, 2021

loganharbour left a comment

Choose a reason for hiding this comment

moosebuild commented Dec 7, 2021

loganharbour commented Dec 7, 2021

loganharbour commented Dec 7, 2021

lindsayad left a comment

Choose a reason for hiding this comment

loganharbour commented Dec 7, 2021 • edited

lindsayad left a comment

Choose a reason for hiding this comment

loganharbour left a comment

Choose a reason for hiding this comment

YaqiWang commented Dec 8, 2021

loganharbour commented Dec 7, 2021 •

edited

moosebuild commented Dec 7, 2021 •

edited

moosebuild commented Dec 7, 2021 •

edited

loganharbour commented Dec 7, 2021 •

edited