DAQInterface should handle double-includes
Earlier this week on ProtoDUNE, problematic runs occurred because an experimenter accidentally double-included a *.fcl file which contained a routing_table_config table definition into his boardreader. The consequence of this is that DAQInterface bookkept the first table but not the second table, and as is standard in FHiCL the second, non-bookkept table definition clobbered the first one.There are a couple of ways DAQInterface's handling of this situation could be improved:
- Make sure that whenever it bookkeeps a variable, be it an atomic, a list or a table, it bookkeeps the last-appearing instance, and not the first-appearing instance
- Have it throw an exception in the event that a variable gets defined twice
Either of these approaches would have prevented this week's headaches, and each has its advantages and disadvantages. The first approach would allow things to "just work", but on the other hand, a doubly-defined variable is a classic instance of "code smell", and the user's error would have been kicked under the rug in this approach. The second approach would cause things to fail loudly rather than quietly, but then the question arises as to whether there are reasonable scenarios where users perform double-definitions.
#1 Updated by John Freeman 6 months ago
- % Done changed from 0 to 100
- Status changed from New to Resolved
This issue is fixed with commit efaf52cac40d05e6d752d184c15de1aa71d71fad on branch bugfix/23725_handle_double_includes. Interestingly, the issue of a non-bookkept variable clobbering a bookkept one is specific to tables involved in process routing. In all other cases, either specific variables are bookkept via the "sub" function from the "re" module (which does global substitution), or we're dealing with a "sources" or "destinations" table, and Kurt put in code to handle the case of multiple such tables in a given file. So, I simply added a check where an exception is thrown if a given routing-related table appears more than once in a file. In the case of the ProtoDUNE issues earlier this week, this change would have caused an immediate, easy-to-spot error rather than a mysterious, hard-to-spot one.