DoNothingBC's and parameter "diff" is being retrieved before being set

[ghost]

Are the DoNothingBC's working properly? I notice that there are no tests available for them in the repository, and I'm trying to use them in an input file but I'm getting an error The parameter "diff" is being retrieved before begin set and I haven't seen this one before.

If they are working properly, please let me know where I might be going wrong such that this error appears. I've attached my input file as a txt file.
DoNothingBC_error_input.txt

[csdechant]

The DriftDiffusionDoNothingBC is old code that was made when @lindsayad was first making Zapdos and hasn't been developed upon since. Looking at the body files, it looks like that BC is not fluid dependent with the material block (i.e. instead of calling the diffusion and mobility coefficient for a specific variable, it calls on a singular mu and diff term). The error is because a material property of mu and diff was not define in the input file. Because multiple variable are using that BC, you need to define the mobility and diffusion coefficient in the each separate BC. It should look something like:

[Species_outlet]
type = DriftDiffusionDoNothingBC
variable = species
mu = #
diff = #
use_material_props = false
boundary = 'outlet'
position_units = ${dom1Scale}
potential = potential
[]
Where # is a Real number.

Edit: Please disregard the above statement. There is a bug that currently requires users to define mu, diff, and sign, regardless if use_material_props = true

[ghost]

Thank you! The error has disappeared and the simulation is running. Not converging yet so I still have some ironing out to do, but good to know the issue is resolvable!

[ghost]

After looking closer at the DriftDiffusionDoNothingBC, I made a mistake on the root of the error. It seems that BC does call variable specific diffusion and mobility, in particular it calls:

_mu(getParam<bool>("use_material_props") ? getADMaterialProperty<Real>("mu" + _var.name()) : _user_mu), _sign(getParam<bool>("use_material_props") ? getMaterialProperty<Real>("sgn" + _var.name()) : _user_sign), _diffusivity(getParam<bool>("use_material_props") ? getADMaterialProperty<Real>("diff" + _var.name()) : _user_diff),

The problem seems to be that regardless if you define the coefficients in the BC or uses the material block, one needs still input a value for diff and mu. This is a bug that needs to be fixed if users are going to use those BC. For @speyres current problem, disregard my previous statement. Set use_material_props = true and set dummy values for diff, mu and sign. As for the electron DoNothingBCs, they should be good since they use the same naming convention as ADGasElectronMoments.

I would like to open this back up because I would like to fix this bug since I am using this BC. As you pointed out, it seems that the issue with the materials is how it calls for the parameters and material properties. I looked through some of the kernels and BCs that are used more frequently and I suggest this may be re-written similarly as such:

    _r_units(1. / getParam<Real>("position_units")),
    _mu(getADMaterialProperty<Real>("mu" + _var.name())),
    _sign(getMaterialProperty<Real>("sgn" + _var.name())),
    _diffusivity(getADMaterialProperty<Real>("diff" + _var.name()))
    _grad_potential(adCoupledGradient("potential"))

This seems to be similar to how it is written in the Electron DoNothingBC pair. What do you think?

[cticenhour]

This would be fine if you didn't want to preserve the _user_* variables. If you did want to preserve that functionality, then you would have to improve the logic in the constructor that sets those variables to some sane defaults when this bug would be encountered. Considering that one can set user-defined materials in the Materials block (like GenericConstantMaterial for example) to essentially replace these variables, I'm in favor of a code simplification.

[ghost]

Great, the simpler the better. I implemented the changes into the BC, and it compiled successfully. It seems the Kernel "DriftDiffusion" also has the same weird material property calling setup, but it doesn't seem like this Kernel is used often.

[cticenhour]

To lend some context, I want to note that this approach is a byproduct of earlier eras of MOOSE where quick and custom material properties were a bit harder to manage, and it often was easier to code in an alternative option for debugging and testing. Not as necessary these days, but that is why this logic exists. Note that variable definitions using the (boolean trigger ? option if true : option if false) syntax is pretty common throughout the framework when creating objects that could apply to various cases.