make CashFlow lazy, introduce notification pass through for observables

[pcaspers]

No description provided.

[pcaspers]

addresses #1748

[coveralls]

coverage: 71.919% (+0.01%) from 71.908% when pulling ba758b8 on pcaspers:make_cashflow_lazy into c08d68c on lballabio:master.

[pcaspers]

@lballabio this is not finished, but before I continue I'd like your opinion on where this is going.

The main work is to add the option to use immutable coupon pricers everywhere. This is the prerequisite (I think) to apply the observation graph compression (= register with observables of an observer instead the observer itself). And we should do this in a backwards compatible way, i.e. the default will remain a coupon pricer that can be changed / set with setPricer(). If we feel that this flexibility is actually not needed, we can deprecate and remove these methods later. This would be a dramatic change though I guess, even for my taste.

We can probably also use the new allowsNotificationPassThrough in other objects to further simplify the observation graph. I'd leave that to later PRs though, we can do this step by step.

[ralfkonrad]

I like the Cashflow being lazy, that's indeed much cleaner.

On the other hand I'm not fully convinced by the immutable coupon pricers everywhere and the allowsNotificationPassThrough.

Looking from the results of this PR we would have a different behavior between

coupon(...);
coupon.setPricer(pricer1);
...
coupon.setPricer(pricer2);    // doesn't throws
...

and

coupon(..., pricer1);
...
coupon.setPricer(pricer2);    // now throws
...

For someone not aware of this PR and the related issue #1748 this is unexpected. If there is setPricer(...) it should work for pricer2 either always or never. But it shouldn't depend on the way we passed pricer1 to the coupon, right?! Someone unaware would have to dig deep into the code to understand what is going on there.

Also passing in a pricer via the constructor or via the setter shouldn't have an impact on the Observer pattern. That's too indirect in my opinion.

And in general I wouldn't tweak the Observer pattern to take care of the observation graph. There shouldn't be some magic aka allowsNotificationPassThrough_ going on which registers a second or third level Observer with an Observable. If there is a business need for it we should directly call observer.registerWith(thirdLevelObservable).

In total, are these pitfalls worth the benefit solving the problem of changing the coupon pricer after assigning the cashflow to an instrument? Isn't it more of an academical problem which doesn't exist in real life? And if someone wants to do this there is still the possibility to register and unregister theses pricers to the instrument directly. So the tools are there to solve this problem.

[pcaspers]

I think the potential compression of the observation graph is too good to ignore: For example, a single vanilla swap with say 40 coupons on each leg registers with 80 observables today. This PR brings that down to 3 observables (index, eval date, coupon pricer).

I don't think it's realistic to do this kind of optimisation in user code even if someone would bother to try. And why should they if we can do this automatically and safely while building the observer graph.

At the moment I don't see a good way to avoid the distinction between immutable and mutable pricers. Of course we should add a comment to the constructors along the lines

"The coupon pricer can be passed to the constructor or set after construction with setPricer(). If passed to the constructor, the pricer is immutable, i.e. a subsequent call to setPricer() will throw an error. This is the recommended way, because it allows for an optimised representation of observability relations."

The advantage of handling the optimisation in Observer:registerWith() is that it works without amending the code in Swapor Bond. It is enough that FloatingRateCoupon knows that this optimisation is possible for its derived classes (if an immutable pricer is used). This way the optimisation automatically works for client instruments that register with cashflows, too.

Maybe more important, it would be unsafe to register with the coupon observerables in Swap or Bond since not all cashflow might support the optimisation. For example, I haven't updates inflation coupons to support it.

Finally, they way it is implemented, it will work recursively.

[lballabio]

Hi @pcaspers, I'll have to do some more thinking about this—I like the idea of optimizing the graph but I'm not sure about the added complexity of the allowsNotificationPassThrough mechanism (and, like Ralph, of the dual behavior depending on when the pricer was passed).

Also, I'm not sure that registerWith should always fall back to registerWithObservables when allowed; that should probably be for the specific observer to decide. E.g., It wouldn't work when the observer is a Handle::Link instance.

Are you at the point when you can run some timing experiment and see what's the effect of these changes?

And maybe there are 2 PRs inside this one? One making cashflows lazy and another for graph optimization?

[lballabio]

Oh, I see you partially answered my questions while I was writing them :)

[pcaspers]

A link would not allow the compression by default, which is correct since it might be relinkable?

Actually we could enable it though otherwise, i.e. if it can't be relinked. Separate discussion probably, since it needs some modifications.

I can split the PR into two.

[pcaspers]

The graph optimization reduces the memory footprint: if we have 100k swaps with 80 cashflows each we avoid storing ~8m shared_ptr each in turn storing 16 byte on a 64 bit system, i.e. 128 MB in total.

[lballabio]

A link would not allow the compression by default, which is correct since it might be relinkable?

I'm not worried with observer.registerWith(link), which would not register with the link's observables—the problem would be with link.registerWith(thing) where thing says that it allows pass through. The link would register with the thing's observables, and when relinked to something else it would unregister with the thing but not with its observables (because unregisterWith, correctly I think, doesn't pass through; if it did there would be other problems). Unless I'm reading the changes in Observer incorrectly, which is also possible :)

[pcaspers]

You are right, I didn't think of this. My answer right now would be that if observer registers with a pass-thorough thing then this forbids unregistering with observables. The only allowed operation is unregisterWithAll(). We should throw and error otherwise.

So in addition to the assumption that a pass-through observable must not change its observables after construction, we need to ensure that any observer registering with such an observable must not remove any of its observables. Except it wants to remove all of them.

In case of Link we can probably switch from unregisterWith(h_) to unregisterWithAll() since a link always has one unique observable only anyway.

But in general that can cause errors where they do not occur today, e.g. when client code tries to unregister a swap with one of its coupons. I don't see many use cases for that at the moment, but it's a breaking change anyway!

[pcaspers]

I can look at some bigger use cases. If we think it's worthwhile maybe we can add a compile-time setting OPTIMIZED_OBSERVABILITY which offers the optimizations and introduces the restrictions as well.

[pcaspers]

will open a new PR for the lazy cashflow part