Fix DetElementCreator plugin

[tmadlener]

BEGINRELEASENOTES

• Make the default return value of DetElementCreator::process non-zero to avoid erroneously raising an exception after finishing.
• Add a test case that checks that things work

ENDRELEASENOTES

I am not entirely sure if the DDDB examples were turned off on purpose, there seem to be quite some test failures in there. It might be easier to move the new test case to somewhere else, I just put it into DDDB because it comes with a compact file that uses the DetElementCreator plugin. I can also move all the "noise" into a separate PR, so that this only contains what is really necessary.

The issue that is fixed by this

Without the (one line) fix, I run into the following issue:

[...]
RunPlugin        ERROR ++ Exception while executing plugin DD4hep_CompactLoader:
		dd4hep: apply-plugin: Failed to execute plugin DD4hep_PlacedVolumeProcessor
dd4hep: with plugin:DD4hep_PlacedVolumeProcessor
dd4hep: Error interpreting XML nodes of type <plugin/>
dd4hep: Error interpreting XML nodes of type <plugins/>
dd4hep: while parsing compact.xml
dd4hep: with plugin:DD4hep_CompactLoader
Application      ERROR geoPluginRun: Uncaught exception: RunPlugin: ++ Exception while executing plugin DD4hep_CompactLoader:
		dd4hep: apply-plugin: Failed to execute plugin DD4hep_PlacedVolumeProcessor
dd4hep: with plugin:DD4hep_PlacedVolumeProcessor
dd4hep: Error interpreting XML nodes of type <plugin/>
dd4hep: Error interpreting XML nodes of type <plugins/>
dd4hep: while parsing compact.xml
dd4hep: with plugin:DD4hep_CompactLoader

The exception is raised here:

DD4hep/DDCore/src/DetectorImp.cpp

Lines 854 to 857 in 2a492e2

	result = *(long*) result;
	if (result != 1) {
	throw runtime_error("dd4hep: apply-plugin: Failed to execute plugin " + fac);
	}

The return value in turn is set here:

DD4hep/DDCore/src/plugins/StandardPlugins.cpp

Lines 1406 to 1411 in 2a492e2

	DetElement det = detail::tools::findElement(description, path);
	if ( det.isValid() ) {
	pv = det.placement();
	if ( pv.isValid() ) {
	return PlacedVolumeScanner().scanPlacements(*proc, pv, 0, recursive) > 0 ? 1 : 0;
	}

Which finally calls PlacedVolumeManager::process

DD4hep/DDCore/src/VolumeProcessor.cpp

Line 33 in 2a492e2

ret += process(placement,level+1,recursive);

The process that is called in this case is DetElementCreator::process. One of the paths through this looks like this

DD4hep/DDCore/src/gdml/DetElementCreator.cpp

Lines 317 to 331 in 2a492e2

	int DetElementCreator::process(PlacedVolume pv, int lvl, bool recursive) {
	int ret = 0;
	string pv_nam = pv.name();
	if ( detector_volume_level > 0 ||
	( (!detector_volume_match.empty() &&
	pv_nam.find(detector_volume_match) != string::npos) &&
	(detector_volume_veto.empty() ||
	pv_nam.find(detector_volume_veto) == string::npos) ) )
	{
	stack.emplace_back(Data(pv));
	if ( 0 == detector_volume_level ) {
	detector_volume_level = stack.size();
	createTopLevelDetectors(pv);
	}
	ret = PlacedVolumeProcessor::process(pv,lvl,recursive);

the other looks like this

DD4hep/DDCore/src/gdml/DetElementCreator.cpp

Lines 422 to 426 in 2a492e2

	else if ( lvl < max_volume_level ) {
	//printout(printLevel, "", "+++ Skip volume %s", pv_nam.c_str());
	ret = PlacedVolumeProcessor::process(pv,lvl,recursive);
	}
	return ret;

If I am not mistaken, this means that even if the DetElementCreator does nothing (i.e. automatic success?), it still returns 0. That at some point signals failure to the machinery. Hence, changing the default return value to 1, would fix this. I am not entirely sure about all the assumptions in this, but this would fix things for me.

[MarkusFrankATcernch]

@tmadlener Did you re-enable DDDB ?
I thought DDDB was not built anymore.
It is entirely obsolete.

[tmadlener]

Ah, I wasn't aware of that. I will then move "my" test to somewhere non-obsolete. Any immediate suggestions for that?

[MarkusFrankATcernch]

The generic container for all tests which are not really logically connected is "ClientTests".

[tmadlener]

I moved the compact and necessary input file to ClientTests and added the test there. I also removed the previous changes to DDDB and also disabled DD4HEP_BUILD_DEBUG again for CI.

[tmadlener]

New test seems to run and succeed, e.g.: https://github.com/AIDASoft/DD4hep/actions/runs/5355697069/jobs/9714291446?pr=1138#step:4:3663

[github-actions]

Test Results

       6 files         6 suites   6h 16m 16s ⏱️
   349 tests    349 ✔️ 0 💤 0 ❌
1 037 runs  1 037 ✔️ 0 💤 0 ❌

Results for commit c78e087.