This now makes most algorithms work on PHYSLITE, and the ones that do
not work can be turned off.

This is now hopefully more straightforward than it was, and should
allow future refactoring as needed.

This removes all the implicit views created by the various sequences,
and instead creates explicit views as inputs for MET and OR.  And it
adds actual consistent thinning for all containers for the output.
And it adds pt/eta cuts for all the objects, which are probably way
off from what they should be...

I also had to add some extra configuration options/decorations to some
of the sequences to make it all consistent.

Tadej Novak authored 2 years ago and Atlas Nightlybuild committed 2 years ago

Properly apply pt cut in analysis sequences

See merge request !57419

(cherry picked from commit 90677e35)

dd72cb76 Properly apply pt cut in analysis sequences

This goes back to an old discussion that in principle AsgSelectionAlg
should not have a SysCopyHandle, as it doesn't introduce new momentum
systematics.  The reason it has one currently is that if there is a
selection before the correction algorithm it does need to make a
shallow copy that can then subsequently be modified at will.

The solution implemented here is what we discussed back then: To
implement an algorithm that does nothing more than make a shallow copy
(based on the view container algorithm and the copy mechanism from
SysCopyHandle) and schedule that before AsgSelectionAlg as needed.

Given that the first iteration of the configuration block design
invoked criticism of the "reference" design for tracking containers I
now changed it, so that instead of first allocating references to
determine what the intermediate containers are, I just run through the
entire configuration twice.  The second run-through then can rely on
what it learned in the first run-through to determine what will happen
further down in the sequence.

This requires routing a number of calls through the ConfigAccumulator
object, but it definitely makes writing the actual blocks more
straightforward, and it should also allow for more flexibility if we
need to redesign the ConfigAccumulator in the future (which also
became simpler with this change).

Nils Krumnack authored 2 years ago and Nils Erik Krumnack committed 2 years ago

This allows testing that software works on DAOD_PHYS.

I'm keeping the old files around with the suffix _OLD, as users have
specifically requested to be able to keep tests on older files which
may require different tool settings.

I also updated a fair number of the CP algorithm tests so that they
now explicitly use the _OLD files until issues are resolved.

This is analogous to SysReadSelectionHandle, but for writing.  I'm not
currently using it for actually writing systematically varied
decorations, but it already helped clean up all the code that was
previously using bare accessors to write selection decorations.

The current users of ISelectionAccessor interface only ever use it to
read or to write a selection decoration, never both, and none of the
implementations actually benefit from doing both in the same class.
Quite the contrary, a lot of the implementations can only do reading
and only provide an error when trying to write to them.  Splitting the
interface into two (and the implementations as needed) makes the code
more clear, and avoids having all the functions in the implementations
that just produce an error because that part of the interface can not
be implemented in the class.

Implementation-wise I split `ISelectionAccessor` into
`ISelectionReadAccessor` and `ISelectionWriteAccessor`, and propagated
that through the code, using the read or write interface as necessary.
Implementations of the interface were changed to implement either only
the read-interface or split into implementations of both interfaces.

The only code that is actually shared between both accessor types is
the `label()` member for debugging (which is duplicated), and the code
that parses the "selection,as_bits" syntax (which I handled via a
`SplitData` helper class).

The idea is to allow reading systematics varied selection decorations
with the existing ISelectionAccessor infrastructure and being fully
integrated with our systematics handles.

I updated all the algorithms to use this, but haven't yet updated the
configuration to use it, and also haven't added any specific tests for
this purpose.

The main goal here is to provide a foundation on which we can build,
and that allows to quickly convert all the existing configurations, so
that we can drop the old configuration quickly and don't have to
maintain two parallel configuration mechanisms over an extended period
of time.

The interfaces and implementations will likely change, particularly as
we evolve the configuration to add another layer on top of it.  As
such the primary goal here is to provide a sort of stable interface
for the actual users of the CP algorithms, so that they don't have to
constantly update as we tune/change this.  I'd also like to keep the
actual structure of the configuration blocks sort of stable, but I'd
expect some degree of changes here over time.

The main ingredients of this configuration model are:
* ConfigBlock: The actual object specific configuration classes
  that have individual implementations for each group of algorithms that
  should be configured as an indivisible unit.
* ConfigSequence: A sequence of blocks, though technically this could
  just be a simple python list instead.
* ConfigAccumulator: A helper object that is used to track all the
  information needed to communicate between (and within) blocks.
* container references: Helper objects that track all the shallow copies
  belonging to the same logical object.  This is taking the place of the
  current post-configuration step, essentially front-loading the
  connections between blocks.

Nils Krumnack authored 3 years ago and Nils Erik Krumnack committed 3 years ago

Essentially the idea here is that this makes it easier to build
additional configuration layers on top, since it makes this more
composable.  In particular it means that multiple working points can
be added to a single sequence without having to do some complicated
special handling to deduplicate the calibration part of the sequence.
I also purposely pass in the actual sequence I operate on instead of
creating them internally, which (hopefully) gives the caller some more
freedom as to what that sequence may look like.

I didn't try to do anything particularly clever in splitting the
sequences up, and some cleanup could be sensible at some point.
Basically I split the sequences into three parts:
* calibration: any algorithms that can be shared between multiple
  working points
* working point: any algorithms that do things specific to a
  a working point that can't be shared between object types
* post processing: any algorithms that can be shared between multiple
  object types (essentially container copies and debugging histograms)

There is one algorithm that could go into either the working point or
the post-processing, and I chose working point: And that's the
algorithm that collects all the decorations for that working point and
makes an overall selection decoration for that working point.  My
reasoning here being that while that is a completely generic algorithm
it is still something we will always want to do and that the working
point sequence will feel incomplete if we leave it out (unlike the
post processing which the user could easily do his own way).  If we
find that is not the case we could move this around later on (or make
it optional).

Also, I found that at least in some places we have two types of
selections: One for the analysis object selection and one for the
output object selection.  For simplicity/consistency I just forced
that on all sequences, but it may be worth revisiting at some point.

I also had to do some amount of reordering to fit into this overall
scheme.  Most notably the e-gamma sequences did one of their WP specific
selections before running calibrations.  I flipped the order (at least
for now), as this is essentially a performance optimization (skipping
calibration for some objects) and we can bring it back later if we
decide that such an optimization is worth the headache it brings with it.

I also updated the photon sequence no longer to use view-copies
internally, but instead rely on preselection decorations.  Since that
is the way we said all sequences ought to have been working already
that seemed like a sensible change.

I also put the preselection configuration into the tau sequences.
Technically that is not used, but it allows pre-loading preselection
algorithms.

I'm not sure how sensible my updates to the di-tau sequence are, but
the original sequence was neither functional nor complete.  The unit
tests for that sequence were already commented out, and the sequence
itself doesn't even contain a selection algorithm.  Still I tried my
best to update it, to reduce future work.

I did leave the original top-level function around, calling the new
functions as they ought to be called.  That is mostly ment for
backward compatibility and it ought to be fine to call the new
functions directly as a user (though they may see more updates to
calling conventions than the top-level function).

Currently that does nothing, but since meta-information might be used
it is probably better to configure than not to configure the sequence.

On request of @jburr to make this easier to understand and teach for
beginners.  Note that this involved a fair amount of whitespace
changes to make the for-body indentation consistent with the rest of
each file.

Essentially when we are using a decoration handle we should not need
to make a copy, but be able to directly decorate the input object.

There are probably other common CP algorithms that could be switched
from `SysCopyHandle` to `SysReadHandle`, but this is the lowest
hanging fruit.

The ultimate goal here is to have the python configurable no longer
inherit from the C++ object, and instead create the C++ object only at
the moment when it is needed, instead of having it as a base class and
performing all updates simultaneously to the python object and the C++
object (to keep them in sync).  However, for now it mostly adds an
alternate way of adding algorithms to jobs, to actually make the more
fundamental change to the configurables would require all users to
update to the new formalism of adding algorithms to jobs.

This also allows additional configurable objects to be used, e.g. I
now treat algorithm sequences and algorithms the same.

This could potentially also be used to change the target or mechanism
of configuration at some point, e.g. to push this into an intermediate
JSON file, but that's not an immediate goal.

For now this just replaces the algorithm with a service, and
communicates the systematics list via the service interface instead of
via the event store.

The plan is to add more functionality in future merge requests that
will move a lot of the systematics handling functionality from the
configuration layer into the service.

Mostly so that it acts as a test of whether the EventLoop python
configuration would work properly on the grid.

This is needed to make them work in the presence of the issues described in
https://sft.its.cern.ch/jira/browse/ROOT-10940.

Martin Errenst authored 4 years ago and Nils Erik Krumnack committed 4 years ago

The list of isolation WPs is taken from
https://gitlab.cern.ch/atlas/athena/-/blob/21.2/PhysicsAnalysis/AnalysisCommon/IsolationSelection/Root/IsolationSelectionTool.cxx#L175

Add optional names

Fix isolation in makeMuonAnalysisSequence tests

Use NonIso for muon CP alg tests

Just for safety, since the lists may be subsequently updated.

This first needed the extension for subtools, but now that that is
taken care of, this should work.

Just to further simplify/streamline the configuration, and make sure
we can reorder algorithms interacting with output algorithms.

This should increase both readability and flexibility (a rare double
win), compared to the old setup.  Let's see if people like it.

It's not used everywhere, but it would (in principle) allow dynamic
reordering/reconfiguration of the muon sequence algorithms, and the
preselection decorations etc. would then pick up that reconfiguration
change.

For now that is just a minor cleanup, but hopefully in the future it
will allow for dynamic reconfiguration/rearranging of sequences.