FrameSummer should be n-way not just 2-way + has vestigial code.

[brettviren]

Problem

As found by @HaiwangYu FrameSummer is strictly a 2-way sum. N-way sum is needed, eg to support schemes to simulate ICARUS non-uniform response.

Proposed solution

It looks like actually it's a simple "upgrade":

1. change FrameSummer base class to IFrameFanin
2. delete all code in FrameSummer related to the align and offset configuration parameters - including newtwo.
3. keep Aux::sum()

Comments

The reason FrameSummer is 2-way in the first place is simply because it was written before we added support for the N-way IFrameFanin.

About (1), this ignored newtwo is "vestigial" code but it likely was never used as existing cfg/ configuration never turned it on.

However it's intention is eventually required. For right now ("chunked" simulation mode) it is safe to assume the time() of input frames are aligned. When we move to "streaming" simulation mode then input frames may not be time-aligned. Either FrameSummer must handle this alignment or it must reject it (eg, throw RuntimeError if a variety of time() values are seen).

It looks like Aux::sum() handles this variety and accepts a vector<IFrame::pointer> so will do the right thing automatically.

Removing align and offset configuration parameters will not break any intention (since they are all align=false or align=true with offset=0) and giving such configuration in the first place was not well thought out.

Reference

This code was introduced back in 2018 and before WCT went to a monorepo. It's commits are seen here:

https://github.com/WireCell/wire-cell-gen/blame/master/src/FrameSummer.cxx#L47

Aux::sum()

https://github.com/WireCell/wire-cell-toolkit/blob/master/aux/src/FrameTools.cxx#L360

[jzennamo]

I am happy to take this on and already have a functioning patch in my feature branch