Project

General

Profile

Bug #23725

DAQInterface should handle double-includes

Added by John Freeman 12 months ago. Updated 11 months ago.

Status:
Reviewed
Priority:
Normal
Assignee:
Category:
-
Target version:
-
Start date:
12/06/2019
Due date:
% Done:

100%

Estimated time:
Experiment:
-
Co-Assignees:
Duration:

Description

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.

Associated revisions

Revision efaf52ca (diff)
Added by John Freeman 12 months ago

JCF: Issue #23725: for the routing tables, add a check that each one doesn't appear twice in a file

History

#1 Updated by John Freeman 12 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.

#2 Updated by Eric Flumerfelt 11 months ago

  • Status changed from Resolved to Reviewed
  • Co-Assignees Eric Flumerfelt added

Code review & demo testing successful.

Also available in: Atom PDF