Unexpected behavior with parsing of config_compilers.xml into Macros.cmake (not overriding defaults)

[ndkeen]

In config_compilers.xml We have compiler-specific cases such as:

<compiler COMPILER="gnu">

where we have all GNU compiler basic flags/settings. If we need to make a change to those settings for a specific machine, my understanding is that we could so something like the following:

<compiler MACH="cori-knl" COMPILER="gnu">
  <ALBANY_PATH>/global/project/projectdirs/acme/software/AlbanyTrilinos20190823/albany-build/install</ALBANY_PATH>
  <CONFIG_ARGS>
    <base> --host=Linux </base>
  </CONFIG_ARGS>
  <CPPDEFS>
    <append> -DARCH_MIC_KNL </append>
  </CPPDEFS>
  <CFLAGS>
    <base DEBUG="TRUE"> -g </base>
    <base DEBUG="FALSE"> -O2</base>
    <append compile_threaded="TRUE"> -fopenmp </append>
    <append COMP_NAME="csm_share"> -std=c99 </append>
  </CFLAGS>
  <FFLAGS>
    <base DEBUG="TRUE"> -g</base>
    <base DEBUG="FALSE">  -O2</base>
    <append compile_threaded="TRUE"> -fopenmp </append>
  </FFLAGS>
...

Here in the example, I have made the changes more visible (ie wanting DEBUG flags to simply be -g for cori-knl with gnu) where in practice my desired change is more subtle.

It also looks any commands of a "section" in config_compilers.xml is simply added to other commands -- such that even now, I see multiple commands in Macros.cmake that were likely not what was intended (and only work as it's a simple flag append), such as:

   set(FFLAGS "${FFLAGS}  -O2 -debug minimal")
   set(FFLAGS "${FFLAGS}  -O2 -debug minimal -qno-opt-dynamic-align")

where the first command is coming from the <compiler COMPILER="intel"> section, and the second is coming from <compiler MACH="cori-knl" COMPILER="intel"> section. Again, it still works, but I would think the expectation is that the settings in <compiler MACH="cori-knl" COMPILER="intel"> would override the other default settings.

Note I see this in both Macros.cmake and Macros.make. But I sure thought this was NOT happening a while back. Would need to go back to older repos to test.

I hope I've made this clear, but can explain a practical issue to be more concrete.

[jgfouca]

@jedwards4b , remind me, who was it that wrote most of our Macro generating code (the stuff in CIME/BuildTools)?

[jedwards4b]

That was @quantheory , he has since moved on to other things. But problems with the cmake output don't surprise me as we have concentrated mostly on make output.

[worleyph]

@jgfouca , note that this looks like the same issue that arose with the Depends files, where you had to re-engineer the non-CMAKE build to preserve the old logic, but decided that you preferred the "append" logic for CMAKE.

[jgfouca]

@worleyph , are you talking about this?

Change FFLAGS_NOPT to be an append to the normal set of flags. This reduces duplication with FFLAGS

[worleyph]

The E3SM side PR was

 Fix Depends-file usage for legacy build system #3350

but I misremembered the details. The effect is to not append multiple Depends files because only one Depends files was used, but it was still appended to the defaults, I guess. Sorry for the irrelevant comment.

[jgfouca]

@jedwards4b , @ndkeen ,

I did some testing on our macro system and I think I understand what's going on.

Let me say first, the behavior noted by @ndkeen is not related to the cmakification of E3SM's build system and that the same behavior exists in the Makefile macros.

Here's what I observed:

Assume the phrase "parent block" refers to the generic compiler block, IE:

<compiler COMPILER="gnu">
...
</compiler>

Assume the phrase "child block" refers to a more-specific compiler+machine block, IE:

<compiler MACH="melvin" COMPILER="gnu">
...
</compiler>

For a flag field, say FFLAGS, if the parent block has both a "base" value and some appends, IE:

  <FFLAGS>
    <base> -fouc1</base>
    <append DEBUG="TRUE"> -fouc2</append>
  </FFLAGS>

And if the child block attempts to override both the "base" and add some appends:

  <FFLAGS>
    <base> -fouc3</base>
    <append DEBUG="TRUE"> -fouc4</append>
  </FFLAGS>

What I'm seeing is that our current system allows the user to overwrite the "base" but there is no way to get rid of the appends from the parent block. In other words, in the example above, '-fouc1' does get replaced with '-fouc3' but there is no way for the child block to remove '-fouc2'. The only solution for the child block is to append a flag that would undo the flag from the parent.

I don't know if @quantheory is available for comment; @billsacks you may have some thoughts as well. Does anyone know if it was a deliberate design decision to have the parent block appends be permanent in all children? Do we need to provide a way to allow a child to define a field in such a way that all parent settings are ignored?

[jedwards4b]

It seems to me that if a child block defines base for a flag then all previous appends for that flag should be removed.

[jgfouca]

@jedwards4b , I think so too and that's clearly the behavior that @ndkeen expected.

[jgfouca]

I just did a quick survey of our (e3sm) config_compilers.xml file and unfortunately the pattern of overriding "base" on a field that has appends in the parent is widespread, so if we changed this behavior (non-backwords compatible change) then we could impact nearly all of our builds.

Not sure what to do at this point.

[ndkeen]

FYI, I checked out some older repos. On June of 2018, our repo seemed to be doing what I would think is the expected behavior. In Feb 2019, it was not. So it's certainly possible this has been an issue for a while.

[jgfouca]

@ndkeen , that matches this commit:

commit 1169bf496b688a828287225e676c52e2158a4b86
Author: Sean Patrick Santos <SeanPatrickSantos@gmail.com>
Date:   Wed Jul 11 11:16:44 2018 -0700

    Rework machine overrides in config_compilers.
    
    In the original design, machine-specific settings would completely
    override all non-machine-specific settings. This led to two unintuitive
    behaviors in particular:
     - A machine-specific setting that does *not* apply could override one
       that does (e.g. because the machine-specific setting only applies to
       cases with a particular MPI library, whereas the non-specific setting
       applies to all cases).
     - A machine-specific `<base>` tag causes all non-machine-specific
       `<append>` tags to be ignored.
    
    This commit changes both behaviors. Now machine-specific settings will
    only override non-specific settings if they actually apply to the
    current case. Furthermore, `<append>` tags can never be overridden,
    regardless of the `<base>` tags that are involved.
    
    While this is expected to have minimal effect on existing
    config_compilers.xml settings, it is quite possible, even likely, that a
    small number of changes will have to be made for some machines.

[ndkeen]

Hmm. Yea, reading that makes me wonder -- is that what we really want? It's just not clear if the intention was to prevent what I'm trying to do or not. And maybe it didn't cause any issues because all machine-specific changes were "adds".

As a work-around, I tried adding a section like this: <compiler COMPILER="gnu" MACH="!cori-knl"> but it doesn't seem to be working. That is, I'm trying to add commands that happen for all machines except cori-knl.

[jgfouca]

As a work-around, I tried adding a section like this: but it doesn't seem to be working. That is, I'm trying to add commands that happen for all machines except cori-knl.

Unfortunately, we don't support negation in these keys.

The only workaround I can think of is to append flags in the child block that undo/override the flags you don't want from the parent block.

[ndkeen]

Can you show an example?