Phalanx: turn PHX_ALLOW_MULTIPLE_EVALUATORS_FOR_SAME_FIELD off by default

[bartgol]

@trilinos/Phalanx

Expectations

I think more likely than not one wants to see an error if two evaluators evaluate the same field. I am ok with keeping the feature (CMakeLists.txt explain it is for backward compatibility), but should not be the default. It is more costly to debug an app that wrongly has two evaluators evaluating the same field, than it is for another app to add the configure option to allow this. The former, can take hours of debugging (I did this mistake twice); the latter takes one failed run (with meaningful error message) to adjust the installation.

Current Behavior

The flag is on by default, so PHX allows multiple evaluators to evaluate the same field, in which case, the first is used, and the others tossed. If one does not want this feature, the option must be explicitly turned off at configure time.

Note: this option was introduced in 2015, and left ON by default for bwd compatibility in existing codes. I believe 3 years is a reasonable time for codes to either adapt, or add a config option to their trilinos configuration.

Motivation and Context

It is not unlikely to fall in the scenario where 2 evaluators evaluate the same field. Perhaps you used to have OldEvaluator to compute F, but now you wrote VeryCoolNewEvaluator to evaluate F. So you add it to your problem, but forget to delete the previous one. Or worse yet: two evaluators happen to evaluate the same field because of a particular clash in your app parameters, in which case, you probably need to rethink your app flow (so you would love to see an error message).

Possible Solution

Simply change the default to OFF. Or remove the option altogether. It may be that it was meant to achieve the same thing that contributed fields now allow. So perhaps this is no longer needed.

Steps to Reproduce

Take your favorite problem, and register the same evaluator twice, and see if you get an error. Yes, in that case everything would be fine, since the two evaluators are the same, but it still exposes the problem: no errors with the default PHX configuration.

[bartgol]

Note: the error message for the case when the option is OFF could also specify that such option exists, so the user knows how to reconfigure trilinos in order to use the feature.

[rppawlo]

I completely agree with you. It is a question of manpower. There is an issue with the Panzer linear object factory that results in multiple registrations of the same evaluator. Until that is redesigned, we can't change the default or panzer will break in every configure of Trilinos. Both Eric and I explored fixing this but we both came to the conclusion that it would take quite a bit of work. It's on the todo list but not high priority. Not sure what to do here as I don't think the timing of cmake configure would allow us to check for panzer being enabled before the phalanx config file is written. Maybe @bartlettroscoe could comment on this?

[rppawlo]

Note: the error message for the case when the option is OFF could also specify that such option exists, so the user knows how to reconfigure trilinos in order to use the feature.

I'll add that to the configure and also any other phalanx options not reported.

[bartgol]

@rppawlo Is panzer the only (known) customer of phalanx that uses that feature? If so, it would be really nice to find a way in cmake/tribits to detect whether panzer is enabled, and set the default accordingly.

Another short term solution could be to print a warning in case the option is ON and two evaluators are registered that evaluate a common field. At least one would know this is happening.

For the short term, I will stick a check in Albany's configuration, to make sure such option is OFF in Phalanx (I'm assuming the option is exported in the cmake installation stuff?).

Thanks!

P.s.: you can do whatever you deem appropriate with this issue (close, or keep open for tracking purposes).

[bartlettroscoe]

@rppawlo and @bartgol,

Exactly what do you want to accomplish? Do you just want to know if Panzer is enabled or not when Trilinos/cmake/phalanx/CMakeLists.txt is being processed? If not what is the use case specifically.

Note, there are few points of specialization that you can put in specialized logic in files that get processed by TriBITS. The full order of processing of files is given in:

• https://tribits.org/doc/TribitsDevelopersGuide.html#full-tribits-project-configuration

[bartgol]

@rppawlo I can't find the option in the installed cmake files. Is it possible to do it? It would help (at least in Albany, where panzer is not used) to at least check at config time if phalanx is installed without that switch ON.

[bartgol]

@bartlettroscoe I think the goal would be to know whether Panzer is enabled or not at the time phalanx/CMakeLists.txt is processed, so that the option we are talking about can be enabled/disabled by default accordingly. In particular, I think the default should be ON if Panzer is enabled, and should be OFF is Panzer is not enabled.

[bartlettroscoe]

I think the goal would be to know whether Panzer is enabled or not at the time phalanx/CMakeLists.txt is processed, so that the option we are talking about can be enabled/disabled by default accordingly.

@bartgol and @rppawlo ,

Can not just check the value of ${${PROJECT_NAME}_ENABLE_Panzer} when inside of the file Trilinos/cmake/phalanx/CMakeLists.txt? As shown here, by the time you process the CMakeLists.txt files for any package you know the full set of TPLs and Packages that are enabled or disabled.

[rppawlo]

ok. I will change the default for non-panzer builds.

[rppawlo]

I can't find the option in the installed cmake files. Is it possible to do it?

It is in the Phalanx_config.hpp file. I will push up a PR shortly.

[rppawlo]

@bartlettroscoe - If I try to use that I get a configure error. Looks like the flag is empty. What does an empty variable imply - on/off/unknown?

CMake Error at packages/phalanx/CMakeLists.txt:15 (TRIBITS_ADD_OPTION_AND_DEFINE):
  TRIBITS_ADD_OPTION_AND_DEFINE Macro invoked with incorrect arguments for
  macro named: TRIBITS_ADD_OPTION_AND_DEFINE


-- Phalanx_ALLOW_MULTIPLE_EVALUATORS_FOR_SAME_FIELD = 

[rppawlo]

nevermind - that is not the correct flag since phalanx comes earlier than panzer. I need to use the project name enable.

[bartlettroscoe]

Yea, you have to use ${${PROJECT_NAME}_ENABLE_Panzer} not ${${PACKAGE_NAME}_ENABLE_Panzer} in this context (and Phalax_ENABLE_Panzer makes no sense since Panzer comes after Phalanx).

[rppawlo]

P.s.: you can do whatever you deem appropriate with this issue (close, or keep open for tracking purposes).

@bartgol - I'm going to close this issue. The defaults is now as you requested unless panzer is enabled.

[bartgol]

Thanks Roger!