GenericParameters should not return a default constructed / empty value if key is not present

[tmadlener]

Currently the GenericParameters return an empty / default constructed value in case a key is not found:

podio/include/podio/GenericParameters.h

Lines 216 to 222 in b94cc63

	const auto it = map.find(key);
	// If there is no entry to the key, we just return an empty default
	// TODO: make this case detectable from the outside
	if (it == map.end()) {
	static const auto empty = T{};
	return empty;
	}

This currently makes it hard to impossible to check whether a value for a given key has been explicitly set to an empty / default value, or whether no value was set for a given key. Arguably for vectors and strings an empty value could indicate an unset value, but for int, float, double the "special value" of 0 might actually be a valid value for some parameters and it should be possible to distinguish that from an unset value.

The most straight forward solution in this case would be to make the return value a std::optional. The main drawback is that std::optional does not allow us to return a const & any longer so we will potentially introduce some more copies. Since set (and setValue) currently allow to reset values, this is also a way to invalidate these const &, so I think returning a copy instead of a const ref would by somewhat desirable from a thread safety point of view.

@wdconinc @nathanwbrei not sure how prevalent the use of these parameters are for the EIC. They are also used for Frame::putParameter and Frame::getParameter, so a change in behavior for the GenericParameters might also be visible when using Frames.

[wdconinc]

I just ran a few searches, and it doesn't seem like we use GenericParameters anywhere. The linked PR should be good to merge from our point of view. Thanks for checking.

[tmadlener]

Just to be sure, if #580 is merged this will also affect the Frame interface as that will also start to return optional for getParameter calls.

[wdconinc]

I confirmed that with #580, the only breakage in our stack is in DD4hep (1.28 and master) at https://github.com/AIDASoft/DD4hep/blob/f08e3359b2538cbae320e9fb9ff51e3eb0d780bb/DDG4/edm4hep/Geant4Output2EDM4hep.cpp#L451 (since no PR yet, I fixed it for now by adding .value_or(1) at the end, though that's not backwards compatible).

[tmadlener]

Thanks for the thorugh check. Let me clean up #580 a bit then and take care of making the change transparent in DD4hep.