Values not substituted in multiple input file context

[joshuahansel]

Bug Description

When using the multiple input file capability

app-opt -i fileA.i fileB.i

substitutions do not occur correctly. See example below.

Steps to Reproduce

fileA.i:

foo = 2

[Mesh]
  type = GeneratedMesh
  dim = 1
[]

# A block is necessary in this file, even if empty, to produce the error
[Postprocessors]
[]

[Problem]
  solve = false
[]

[Executioner]
  type = Steady
[]

fileB.i:

bar = 3
val = ${fparse foo + bar}

[Postprocessors]
  [test_pp]
    type = ConstantPostprocessor
    value = ${val}
  []
[]

This gives the error:

*** ERROR ***
fileB.i:7.5: invalid float syntax for parameter: Postprocessors/test_pp/value=${fparse foo + bar}
fileB.i:7.5: invalid float syntax for parameter: Postprocessors/test_pp/value=${fparse foo + bar}

Impact

There is a workaround: duplicating input files, but this is not a long-term solution.

[joshuahansel]

@MengnanLi91 Can you try this example with your parser to see that it resolves the error?

[MengnanLi91]

@MengnanLi91 Can you try this example with your parser to see that it resolves the error?

Sure,I'll try and let you know

[MengnanLi91]

Still have the error, but now only get one error message instead of two. I think fparse not working properly here

*** ERROR ***
fileB.i:7.5: invalid float syntax for parameter: Postprocessors/test_pp/value=${fparse foo + bar}

[joshuahansel]

@dschwen Any idea what's going on here?

[joshuahansel]

@MengnanLi91 are you going to try and include a fix to this with your parser PR, or will it be a subsequent change? The workaround for this bug is a little ugly - I need to manually do all of my fparses in the secondary input file(s).

[MengnanLi91]

@MengnanLi91 are you going to try and include a fix to this with your parser PR, or will it be a subsequent change? The workaround for this bug is a little ugly - I need to manually do all of my fparses in the secondary input file(s).

I'll look into this. My current PR only includes the parameter duplication check. This issue may need more than that.

[MengnanLi91]

I found a slight change to fileB.i as below works out fine with no error, but value = ${val} gives the error. For some reason, when two block is merged, the parser only does one variable replacement instead of two, so we have ${fparse foo + bar} instead of the real value. The piece I don't understand is why this only happens when there are blocks to merge(If [Postprocessors] is removed from fileA.i, value = ${val} works fine too). My guess it may relate to how the hit tree structure looks like when block merges

[Postprocessors]
  [test_pp]
    type = ConstantPostprocessor
    value = ${fparse foo + bar}
  []
[]

[brandonlangley]

@joshuahansel -

I wonder if this is related to this discussion we had on Slack in October about a similar brace substitution situation.

I have copied and pasted the conclusion I came to then regarding that case here:

---

Looking around a bit, I found this comment on the original brace-expression implementation PR that says:

Brace expressions are evaluated in input-file order (i.e. lexical order) from top to bottom of the input file.

which seems to mean for any substitution-B that depends on another substitution-A, B must be evaluated later in the file than A.

So an example like this will work fine because there's only a single substitution happening:

[Functions]
  [test_fn]
    type = ConstantFunction
    value = ${value_to_use_directly}
  []
[]

value_to_use_directly = 6.0

And something like this will also work fine since the earlier substitution feeds the later one:

value_to_use_directly = ${value_to_use_indirectly}

[Functions]
  [test_fn]
    type = ConstantFunction
    value = ${value_to_use_directly}
  []
[]

value_to_use_indirectly = 6.0

However, something like this will not work as the later substitution will not be evaluated until after it's used above.

value_to_use_indirectly = 6.0

[Functions]
  [test_fn]
    type = ConstantFunction
    value = ${value_to_use_directly}
  []
[]

value_to_use_directly = ${value_to_use_indirectly}

---

Since this only happens if fileA.i has [Postprocessors], after that block is merged I wonder if the input becomes:

foo = 2

[Mesh]
  type = GeneratedMesh
  dim = 1
[]

[Postprocessors]
  [test_pp]
    type = ConstantPostprocessor
    value = ${val}
  []
[]

[Problem]
  solve = false
[]

[Executioner]
  type = Steady
[]

bar = 3
val = ${fparse foo + bar}

where combining of the [Postprocessors] blocks merges the bottom one into the top one moving it above this:

bar = 3
val = ${fparse foo + bar}

If this is the case, then that would effectively put the input in the third scenario from our conversation above.

And the earlier value = ${val} will not work here as it relies on val = ${fparse foo + bar} which now comes later.

In other words, the later val = ${fparse foo + bar} would not be evaluated until after value = ${val} uses it earlier.

I just ran across this issue, so this is all guess-work based on our earlier conversation without looking back at the code.

But the braces "evaluated in input-file order from top to bottom" combined with the block merge may give some insight.

---

EDIT:

It seems running moose/framework/contrib/hit/hit merge -output - fileA.i fileB.i confirms this merge behavior.

It shows Postprocessors/test_pp/value = ${val} is moved above val = ${fparse foo + bar} in the merged input.

[joshuahansel]

@brandonlangley Thanks for looking into this! It's making some sense at least. I still don't understand why it gives the error

fileB.i:7.5: invalid float syntax for parameter: Postprocessors/test_pp/value=${fparse foo + bar}

instead of complaining that val just doesn't exist (yet). So what's the order of operations here? From what I see in Parser.C, it looks like:

1. Merge inputs
2. Substitute ${val}
3. Evaluate commands like ${fparse blah}

It seems like 1 and 2 are switched, but 3 just doesn't happen for some reason. I'm very confused.

[joshuahansel]

I think the right solution is:

1. Combine inputs without merging
2. Substitute ${val}
3. Evaluate commands like ${fparse blah}
4. Merge within the combined input

[MengnanLi91]

I am not sure if we can do combine inputs without merging. The combine and merge are done together in hit::merge() now. When we do combine inputs without merging, we may end up having something like below: Should we allow this or when should we error out? This means fileA.i val will have different values when running individually and together with B. Depend on where fileB.i val is used, fileB.i val may changed after combining as well

fileA.i 
foo = 2
val = ${fparse foo + 3}

fileB.i
foo = 1
bar = 3
val = ${fparse foo + bar}

Combined
foo = 2
val = ${fparse foo + 3}

foo = 1
bar = 3
val = ${fparse foo + bar}

[joshuahansel]

Good point, app-opt -i fileA.i fileB.i allows for overriding, so we should preserve that. So this mythical "combination" of mine would actually need to merge/override the (nodes?) that already exist and then put the remainder at the bottom of fileA.i.

As far as being able to do this "combine" thing, how is it done with !include? I see this as a "combination" but at a point in the file that we choose, and for !include we are not allowing duplicates.

[brandonlangley]

@joshuahansel -

1. Combine inputs without merging
2. Substitute ${val}
3. Evaluate commands like ${fparse blah}
4. Merge within the combined input

Just wanted to note to simplify things for the sake of the conversation, fparse can be removed from the equation.

I threw together a little test utility that simply

1. parses an input file
2. evaluates all brace expressions in the same way that Parser.C currently does
3. then renders the input file

I tested with input files that only use regular brace expression substitution without using fparse.

---

This input file:

foo = 2

[Mesh]
  type = GeneratedMesh
  dim = 1
[]

val = ${foo}

[Postprocessors]
  [test_pp]
    type = ConstantPostprocessor
    value = ${val}
  []
[]

[Problem]
  solve = false
[]

[Executioner]
  type = Steady
[]

renders as this after the current Parser.C flow of brace expression evaluations:

foo = 2

[Mesh]
  type = GeneratedMesh
  dim = 1
[]

val = 2

[Postprocessors]
  [test_pp]
    type = ConstantPostprocessor
    value = 2
  []
[]

[Problem]
  solve = false
[]

[Executioner]
  type = Steady
[]

You see Postprocessors/test_pp/value = 2 works since the earlier val = ${foo} feeds the later value = ${val}.

---

However, this input file:

foo = 2

[Mesh]
  type = GeneratedMesh
  dim = 1
[]

[Postprocessors]
  [test_pp]
    type = ConstantPostprocessor
    value = ${val}
  []
[]

val = ${foo}

[Problem]
  solve = false
[]

[Executioner]
  type = Steady
[]

renders as this after the current Parser.C flow of brace expression evaluations:

foo = 2

[Mesh]
  type = GeneratedMesh
  dim = 1
[]

[Postprocessors]
  [test_pp]
    type = ConstantPostprocessor
    value = ${foo}
  []
[]

val = 2

[Problem]
  solve = false
[]

[Executioner]
  type = Steady
[]

And Postprocessors/test_pp/value = ${foo} doesn't work as the earlier value = ${val} needs the later val = ${foo}.

---

So there is nothing special about fparse and this will happen for any brace expressions after that upward merge.

If it merged the earlier block into the later block instead of the later block into the earlier block, this case should work.

But there are probably other override reasons that blocks need to be merged in that direction that I'm not familiar with.

I didn't write the original block merge / override logic so someone like @dschwen would maybe need to chime in.

[dschwen]

Ok, we need to run the substitutions on each input before merging into the preceeding inputs. And that requires to hold onto the map of global variables from prior inputs. I'll check if that can be done. Basically we'd run a substitution in the second input with the knowledge of val from the substitution run on teb first input.

[joshuahansel]

Reminder for @dschwen checking if this can be done:

And that requires to hold onto the map of global variables from prior inputs.

[joshuahansel]

Would anyone be able to attempt the fix to this?

@dschwen You said

we need to run the substitutions on each input before merging into the preceding inputs.

but wouldn't this have this problem?: if I have foo = 2 and a substitution ${foo} in fileA.i and then override foo = 3 in fileB.i, then I'd end up with the value 2, not the value 3.

Would it be possible to change the merge operation to preserve the order of the nodes in the root being merged in, so that we'd end up with

foo = 2

[Mesh]
  type = GeneratedMesh
  dim = 1
[]

bar = 3
val = ${fparse foo + bar}

[Postprocessors]
  [test_pp]
    type = ConstantPostprocessor
    value = ${val}
  []
[]

[Problem]
  solve = false
[]

[Executioner]
  type = Steady
[]

[brandonlangley]

@joshuahansel -

change the merge operation to preserve the order of the nodes in the root being merged in

With your original example app-opt -i fileA.i fileB.i, I assume the "root being merged in" is fileB.i in this case.

If the merge is changed to preserve the order of nodes from the latter file being merge in, what should happen if you run:

app-opt -i fileB.i fileA.i

instead of:

app-opt -i fileA.i fileB.i

with the same contents of your original fileA.i and fileB.i inputs?

When not overriding parameter values, is the expectation that these should work the same regardless of the file ordering?

app-opt -i fileA.i fileB.i vs app-opt -i fileB.i fileA.i

[joshuahansel]

My expectation is that it would not work the same because if you had foo = 3 in the first and then used ${foo} in the second, it may not work if you did fileB.i fileA.i. However, I don't know if we care about this property or not.

Maybe it would be better to just treat the global variables specially during the merge: just always put them at the top?

[joshuahansel]

My vote is that we do not need to worry about having the property that when not overriding values, app-opt -i fileA.i fileB.i matches app-opt -i fileB.i fileA.i, since we don't expect them to match when we are overriding values either.

So my current suggestion is still: change the merge operation to preserve the order of the nodes of the root being merged in, as in the example above.

Any thoughts/objections to this plan?

[brandonlangley]

@joshuahansel -

I took a stab at this and changed the merge logic so any block that exists in both inputs is moved to the end of its siblings.

So with your original input setup:

fileA.i:

foo = 2

[Mesh]
  type = GeneratedMesh
  dim = 1
[]

[Postprocessors]
[]

[Problem]
  solve = false
[]

[Executioner]
  type = Steady
[]

fileB.i:

bar = 3
val = ${fparse foo + bar}

[Postprocessors]
  [test_pp]
    type = ConstantPostprocessor
    value = ${val}
  []
[]

the result after merging is now:

foo = 2

[Mesh]
  type = GeneratedMesh
  dim = 1
[]

[Problem]
  solve = false
[]

[Executioner]
  type = Steady
[]

bar = 3
val = '${fparse foo + bar}'

[Postprocessors]
  [test_pp]
    type = ConstantPostprocessor
    value = ${val}
  []
[]

where the [Postprocessors] block is placed at its latter location and not the first fileA.i location like it was previously.

I also tested moose_test-opt -i fileA.i fileB.i after this merge update and it runs successfully instead of failing with:

fileB.i:7.5: invalid float syntax for parameter: Postprocessors/test_pp/value=${fparse foo + bar}

If you don't see an issue with this strategy, I'll go ahead and put it on a pull request and ask you and @dschwen to review.

[joshuahansel]

Great! The logic seems sound to me. Thanks once again! I'd be happy to review that PR, and I'll let @dschwen decide if he wants to additionally review

[brandonlangley]

@joshuahansel -

After running all of the framework tests locally, I just saw I have nine tests failing after this update that passed previously.

I believe six of these are expected errors being different due to input order changing by command line argument merges.

But three of the failures are these EXODIFF tests that I haven't looked into, I wonder if input order could be affecting this.

transfers/general_field/nearest_node/regular.2d_overlay/projection_needed_receiving ......... FAILED (EXODIFF)
transfers/general_field/user_object/subdomain.2d_overlay/projection_needed_receiving ........ FAILED (EXODIFF)
transfers/general_field/shape_evaluation/subdomain.2d_overlay/projection_needed_receiving ... FAILED (EXODIFF)

I'm going to run the set of modules tests locally to see what the damage is there.

[joshuahansel]

I looked at projection_needed_receiving. No includes, just CLI args. I tried manually doing the process you described to the test, and it works for me even with the new order. So I'd be curious to compare what those input files look like after the merges to compare my manual ordered input files. Is that something we can do - print the input file after all merges? Note in this case there's a multiapp, so there's a main input file and a sub input file.

[brandonlangley]

@joshuahansel -

For the regular.2d_overlay/projection_needed_receiving failure, I printed the input after both the old and new merges.

For reference, here is the full command running that test showing all the command line parameters that are being merged:

/path/to/moose/test/moose_test-opt -i main.i
  Outputs/file_base=projection_receive
  Mesh/second_order=true
  GlobalParams/bbox_factor=1.5
  AuxVariables/from_sub/order=SECOND
  AuxVariables/from_sub_elem/order=SECOND
  MultiApps/sub/cli_args='Mesh/second_order=true;AuxVariables/from_main/order=SECOND;AuxVariables/from_main_elem/order=FIRST'
  --error-override --no-gdb-backtrace

Some top-level blocks are reordered in main.i and sub.i with the new merge strategy from the block moves to the end.

But perhaps more suspicious, the AuxVariables of both main.i and sub.i are in a different order using the new merge.

I know nothing about how this is supposed to work regarding where input order might matter or where it should not matter.

But here is the ordering of the AuxVariables in both main.i and sub.i with the normal / old merge strategy that passes.

main.i:

[AuxVariables]
  [from_sub]
    initial_condition = -1
    order = SECOND
  []
  [from_sub_elem]
    order = SECOND
    family = MONOMIAL
    initial_condition = -1
  []
  [to_sub]
    [InitialCondition]
      type = FunctionIC
      function = '1 + 2*x*x + 3*y*y*y'
    []
  []
  [to_sub_elem]
    order = CONSTANT
    family = MONOMIAL
    [InitialCondition]
      type = FunctionIC
      function = '2 + 2*x*x + 3*y*y*y'
    []
  []
[]

sub.i:

[AuxVariables]
  [from_main]
    initial_condition = -1
    order = SECOND
  []
  [from_main_elem]
    order = FIRST
    family = MONOMIAL
    initial_condition = -1
  []
  [to_main]
    [InitialCondition]
      type = FunctionIC
      function = '3 + 2*x*x + 3*y*y*y'
    []
  []
  [to_main_elem]
    order = CONSTANT
    family = MONOMIAL
    [InitialCondition]
      type = FunctionIC
      function = '4 + 2*x*x + 3*y*y*y'
    []
  []
[]

And here is the AuxVariables ordering of main.i and sub.i with this new "move to the end" merge strategy which fails.

main.i:

[AuxVariables]
  [to_sub]
    [InitialCondition]
      type = FunctionIC
      function = '1 + 2*x*x + 3*y*y*y'
    []
  []
  [to_sub_elem]
    order = CONSTANT
    family = MONOMIAL
    [InitialCondition]
      type = FunctionIC
      function = '2 + 2*x*x + 3*y*y*y'
    []
  []
  [from_sub]
    initial_condition = -1
    order = SECOND
  []
  [from_sub_elem]
    order = SECOND
    family = MONOMIAL
    initial_condition = -1
  []
[]

sub.i:

[AuxVariables]
  [to_main]
    [InitialCondition]
      type = FunctionIC
      function = '3 + 2*x*x + 3*y*y*y'
    []
  []
  [to_main_elem]
    order = CONSTANT
    family = MONOMIAL
    [InitialCondition]
      type = FunctionIC
      function = '4 + 2*x*x + 3*y*y*y'
    []
  []
  [from_main]
    initial_condition = -1
    order = SECOND
  []
  [from_main_elem]
    order = FIRST
    family = MONOMIAL
    initial_condition = -1
  []
[]

Notice the AuxVariables for from_* were moved after the to_* since these command line parameters merged them in.

  AuxVariables/from_sub/order=SECOND
  AuxVariables/from_sub_elem/order=SECOND
  MultiApps/sub/cli_args='AuxVariables/from_main/order=SECOND;AuxVariables/from_main_elem/order=FIRST'

Both inputs have more reordering, but I figured I'd first ask about AuxVariables first and have no idea if that might matter.

If block or sub-block order can matter sometimes, then I wonder if a better strategy may be this other idea you mentioned:

Maybe it would be better to just treat the global variables specially during the merge: just always put them at the top?

[brandonlangley]

@joshuahansel -

After a lot of time spent testing many scenarios, I have concluded input order does affect the outcome in various cases.

---

I verified my assumption above about reordering of AuxVariables in main.i and sub.i causing that EXODIFF failure.

You can quickly see this one example of input order mattering simply by:

• moving [to_sub] and [to_sub_elem] to the top of the [AuxVariables] block in test/tests/transfers/general_field/nearest_node/regular/main.i
• moving [to_main] and [to_main_elem] to the top of the [AuxVariables] block in test/tests/transfers/general_field/nearest_node/regular/sub.i
• running ./run_tests --re=nearest_node/regular.2d_overlay/projection_needed_receiving
• seeing this reorder of AuxVariables makes the projection_receive_sub1_out.e EXODIFF fail

For projection_receive.e, projection_receive_sub0_out.e, projection_receive_sub2_out.e, the EXODIFF passes.

I haven't looked further into if the failure is caused by the reordering in only one or both of the main.i and sub.i files.

---

You can see a second quick example of an input order dependency that this merge strategy revealed by:

• moving [reporter_forward] to above [file] in the [Positions] block in test/tests/positions/creating_multiapps/apps_from_positions.i
• running ./run_tests --re=positions/creating_multiapps.initial_positions
• seeing this reorder of Positions makes the (previously passing) run fail with this error: Unable to locate Reporter context with name: file/positions_1d

---

Those are only two examples of order dependencies that the framework and modules tests revealed with block moving.

There are many failures I have not looked into, but I think it is safe to say that reordering blocks in the merge is not wise.

As I mentioned, I might take a look at this other idea you had if I have time to see if it could perhaps be a better strategy.

Maybe it would be better to just treat the global variables specially during the merge: just always put them at the top?

Another thought I had was trying to just move the top-level blocks when merging instead of moving sub-blocks around.

But I'm not sure how much time I can spend on it right now unless it's a high priority, @dschwen may have an idea too?

[joshuahansel]

Thanks for investigating this. I feared that input order might be having an impact. I wasn't aware of the ones you've encountered, but I've seen something like this happen before. This sounds like possible bug(s) that the MOOSE team needs to fix. My suggestion would be to wait for us to try and address the order dependency bugs before you invest any more time. When we get that fixed, or if we find that order dependency is by design for some reason (I can't think of why it would be), then we can think on other merge solutions.

[joshuahansel]

Tried your first example. All you need to see an exodiff failure is move [to_main] in the sub input file.

[joshuahansel]

@GiudGiud I'm looking at your nearest_node/regular.2d_overlay/projection_needed_receiving test (see above). It seems there is a warning about equidistant points. The test has allow_warnings = True. Do you see any possibility that with different aux variable ordering in the sub, different values could be chosen?

[GiudGiud]

If there are equidistant points, the result of the transfer is indeed ill-defined. So anything goes, a round-off error in one or the other direction can change the output of the test. But we have been consistently rounding the same way before so the test has not needed maintenance.

AuxVariable ordering though, I don't know how much that would matter

[joshuahansel]

Maybe AuxVariable ordering could cause a change in round-off? But

transfers/general_field/user_object/subdomain.2d_overlay/projection_needed_receiving

does not have an equidistant warning, so I guess it's not necessarily related to that. It's interesting that the 3 diffs are projection_needed_receiving tests.