Proposal: Propagation with range-based for loop

[andiwand]

This is a proposal for an alternative propagation mechanism using range-based for loop.

The idea is to invert the control flow we force on users of the current propagation, where an Actor has to be implemented and is essentially decoupled from the calling site of the propagation:

struct Actor {
  void act() {
    // do stuff
  }
  // remember stuff
};

void fit() {
  Propagator p;
  Options<Actor> o;

  Result r = p.propagate(o);
  // extract stuff remembered by `Actor`
}

IMO that has the big downside that variables have to be moved into the Actor, the act function is called for each step and cannot hold on to information which pushes it to mutable member variables, the important information at the end needs to be extracted from the propagation state to be useful to the user.
Apart from that error handling gets complicated as it has to be passed through multiple layers to end back at the calling site.

The proposed alternative is to use a range-based for loop to do the propagation steps and then have the client code simply inside the for loop:

void fit() {
  Propagator p;
  Options o;

  // remember stuff
  for (auto s : p.propagate(o)) {
    // do stuff
  }
  // no extraction needed
}

The propagation loop could be either step by step or surface by surface. Errors can be dealt with directly on the caller site by inspecting the step result s.

At first, both mechanisms can coexist as they do not exclude each other. If we converge on this newly proposed mechanism we could get rid of Actors entirely which would make quite some templating obsolete. Thinking further ahead we could push path limit abortion, target surface, and maybe even material interaction directly into the Propagator as this is very common functionality.

--- END COMMIT MESSAGE ---

[benjaminhuth]

This would be amazing if we can show this has no compute performance disadvantages! (For which I don't see a reason)

I feel like that a lot of complicated code could go in the long term with this.

The only thing that I do not see immediately benefitting is the CKF implementation with branching, but maybe that could be rather elegantly solved by using the propagation iterators?

std::vector<PropIterator> branches;
Propagator p;

// ...
while(!branches.empty()) {
	auto begin = branches.back();
	for(auto it = begin; it != propagator.end(); ++it) {
		// ...
		branches.push_back(it);
	}
	branches.remove(begin);
}

[github-actions]

📊: Physics performance monitoring for 149c349

Full contents

physmon summary

• ✅ Particles fatras
• ✅ Particles geant4
• ✅ Particles ttbar
• ✅ Vertices ttbar
• ✅ Truth tracking (KF)
• ✅ Truth tracking (GSF)
• ✅ Truth tracking (GX2F)
• ✅ Truth tracking (KF refit)
• ✅ Truth tracking (GSF refit)
• ✅ CKF finding performance | trackfinding | single muon | truth smeared seeding
• ✅ CKF fitting performance | trackfinding | single muon | truth smeared seeding
• ✅ CKF track summary | trackfinding | single muon | truth smeared seeding
• ✅ Seeding trackfinding | single muon | truth estimated seeding
• ✅ CKF finding performance | trackfinding | single muon | truth estimated seeding
• ✅ CKF fitting performance | trackfinding | single muon | truth estimated seeding
• ✅ CKF track summary | trackfinding | single muon | truth estimated seeding
• ✅ Seeding trackfinding | single muon | default seeding
• ✅ CKF finding performance | trackfinding | single muon | default seeding
• ✅ CKF fitting performance | trackfinding | single muon | default seeding
• ✅ CKF track summary | trackfinding | single muon | default seeding
• ✅ Seeding trackfinding | single muon | orthogonal seeding
• ✅ CKF finding performance | trackfinding | single muon | orthogonal seeding
• ✅ CKF fitting performance | trackfinding | single muon | orthogonal seeding
• ✅ CKF track summary | trackfinding | single muon | orthogonal seeding
• ✅ Seeding trackfinding | 4 muon x 50 vertices | default seeding
• ✅ CKF finding performance | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ CKF fitting performance | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ CKF track summary | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ Ambisolver finding performance | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ IVF notime | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ AMVF gauss notime | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ AMVF grid time | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ Seeding trackfinding | ttbar with 200 pileup | default seeding
• ✅ CKF finding performance | trackfinding | ttbar with 200 pileup | default seeding
• ✅ CKF fitting performance | trackfinding | ttbar with 200 pileup | default seeding
• ✅ CKF track summary | trackfinding | ttbar with 200 pileup | default seeding
• ✅ Ambisolver finding performance | trackfinding | ttbar with 200 pileup | default seeding
• ✅ ML Ambisolver | trackfinding | ttbar with 200 pileup | default seeding
• ✅ AMVF gauss notime | trackfinding | ttbar with 200 pileup | default seeding
• ✅ AMVF grid time | trackfinding | ttbar with 200 pileup | default seeding
• 🔵️ Comparison - Truth tracking (GX2F vs KF)

[andiwand]

after a discussion with @benjaminhuth

• the first entry should reflect the initial state
  • an error during initialization can be passed as first element
• the entry should be packed into an Acts::Result to make the error handling mandatory
• the propagation state could/should be carried by the iterator
  • ✔️ sticks closer to the iterator principle and enables meaningful copies
  • ❌ if the iterator generally owns the state, further de-templating seems to get more challenging
• further de-templating has to work with the multi stepper and the GSF
  • maybe by subclassing Propagation?
• a track filter could be implemented as a std::ranges::filter_view LOL

[paulgessinger]

Hm do you think this would actually be simpler, once we get into the details and you need to hide more and more stuff inside the iterator?

This seems like an enormous change, so as usual I would say this has to be worth the effort to make sense.

[andiwand]

Hm do you think this would actually be simpler, once we get into the details and you need to hide more and more stuff inside the iterator?

I think so yes - but to be seen of course. What do you mean by hide more and more stuff inside the iterator? As it is for now it can already be used to implement a KF.

This seems like an enormous change, so as usual I would say this has to be worth the effort to make sense.

Just having this mechanism does not seem like a big change to me. Moving all the algorithms is a bigger effort. But I think it will be ultimately worth it as the actor mechanism is very hard to reason about and to debug.

[paulgessinger]

I've only had a quick look at the diff, so not sure about anything yet.

What I mean is that the propagation is used by everything and by clients.

We'll definitely need to keep the basic propagate method, but I suppose that can be implemented in terms of this paradigm.

In my mind something like skipping steps, or cases where we recover from error would be not obvious, but maybe this works by yielding the error in the loop.

If in the end this is not as much work to change the fitters and track finder, I guess it could work.