EDM design considerations

[GerhardRaven]

A suggestion (feel free to reject it) for the workshop (and after it) is to draw up
a ‘design guidelines’ for the various topics. This is what has already started here.

Let me add my three favourite ‘commandments’:

1. EDM classes shall be efficiently move-constructible
2. EDM classes shall be composable
3. EDM classes shall be immutable once made available

rationale:

2. is driven by 3)
3. is driven by sanity, and making the system 'understandable' (and multi threading friendly)
4. will allow producers to ‘pass by value’ and we avoid explicit heap storage.

If you look at GaudiAlg/GaudiAlg/TransformAlgorithm.h (assuming you’ve run
the script I sent out yesterday -- I will add a pull request for it) you will see a clear
example of why I want 1), and the impact it has (i.e. simplifies things) if you look at eg.
Tf/PatAlgorithms/src/PatMatch.cpp and Calo/CaloReco/src/CaloSinglePhotonAlg.cpp
which are two examples of code adapted to use TransformAlgorithm...

[GerhardRaven]

Maybe another one:

4. EDM classes shall be concrete (i.e. 'final')

We currently use (some) inheritance in the EDM for two reasons:
a) to store objects in the TES -- this can be done with type-erasure instead (a-la std::any)
b) to extend objects -- this can be done by composition instead, provided 2) is satisfied

[betatim]

Can we implement these constraints via the GSL (and the tools that support it)?

[betatim]

(I am obviously in favour of straight jackets that help us not to stray from the true path)

[GerhardRaven]

Interesting suggestion! Not directly the GSL, but if the C++ Core Guidelines become 'extendable' and the static analysers will support 'customisable guideline profiles' (as seems to be suggested) then, eventually, I would expect the answer is 'yes'.

[bcouturi]

Interesting indeed. This needs to be there early on, to make sure the rules are actually applied. Gerhard do you have c C++ AST translation of those requirements ?

[GerhardRaven]

Unfortunately, no... Note that I do know how to write a check for 4) today: just try to inherit from an EDM class, and check that the compilation fails... 3) is hard, as it not a property of the EDM class, but of its usage, so a constraint on the event store methods. 1) is a performance requirement, and 2) is maybe not a property of an individual class...

So automatic 'machine verification' may be difficult -- but I invite everyone to prove me wrong!

[betatim]

I think if we can from the start provide a recipe for automatic checking we
are winning. If it requires a human/cooperation then rules will be useless.

Once you have a working suite for automated testing (that is very easy to
run and extend) adding new tests becomes cheap. It has to be so cheap that
I prefer using the automated test to check if a bug I found has gone away
than my personal script. This does mean you need a high quality setup for
testing:

• easy to add a new test
• has to be fast to run (subsets of tests)

On Thu, Nov 12, 2015 at 11:09 AM GerhardRaven [email protected]
wrote:

Unfortunately, no... Note that I do know how to write a check for 4)
today: just try to inherit from an EDM class, and check that the
compilation fails... 3) is hard, as it not a property of the EDM class, but
of its usage, so a constraint on the event store methods. 1) is a
performance requirement, and 2) is maybe not a property of an individual
class...

So automatic 'machine verification' may be difficult -- but I invite
everyone to prove me wrong!

—
Reply to this email directly or view it on GitHub
#19 (comment)
.

[GerhardRaven]

3. can be machine checkable (but as a requirement on the event store!) as it is 'just a matter of const correctness'. So two out of four sofar. And 2) may be possible at a slightly higher level, namely to actually write a skeleton which actually composes the object in question in some reasonable way. Or maybe that this becomes an implicit side-effect of requiring eg. the 'FCC YAML' description ;-). And 1) can be done by just doing a move construction, but that won't tell whether it is 'efficient', just that 'it is possible. So maybe three and a half out of four...

[GerhardRaven]

So maybe we add

0. requirements on EDM classes should be machine verifiable

(note the use of 'should' and not 'shall' ;-)