fix: set materials-map string constant with path to materials map
Briefly, what does this PR introduce?
This works with https://github.com/eic/EICrecon/pull/826 to set the materials-map string constant in the geometry so we can avoid having to assume that calibrations/materials-map.cbor is where we want the materials map to be searched for. The latter approach leads to race conditions when the same geometry directory is used by multiple running processes which keep updating the calibrations/materials-map.cbor file with what they want, only to read it later when it might have changed again already.
What kind of change does this PR introduce?
- [x] Bug fix (issue: possible cause for craterlake eicweb reconstruction benchmark failures)
- [ ] New feature (issue #__)
- [ ] Documentation update
- [ ] Other: __
Please check if this PR fulfills the following:
- [ ] Tests for the changes have been added
- [ ] Documentation has been added / updated
- [x] Changes have been communicated to collaborators: @ShujieL @bschmookler
Does this PR introduce breaking changes? What changes might users need to make to their code?
No.
Does this PR change default behavior?
Yes. The materials map will be written to a different file name. An updated EICrecon will be required to pick it up automatically. Alternatively, you can explicitly pass the option e.g. -Pacts:MaterialMap=calibrations/materials-map.cbor.
I have a couple questions:
- Can we add the value from the plugin itself, using https://github.com/AIDASoft/DD4hep/blob/ad3c96d6ff95dca60d510fe78f0cffa25068c81c/DDCore/include/DD4hep/DetectorImp.h#L344 The downside is that it possibly creates a precedent of a constant not defined in xml.
- Instead of name suffixes like
-archeswould it make more sense to use content-addressed filenames (include file hash in the name)?
Can we add the value from the plugin itself
I think you already hint at the reason: it puts more stuff inside the plugins, which I'd rather not do because it hides it from where users would naturally go and look for this info.
@wdconinc Is this ready to merge?
Nah, let's keep it for next month, if at all.
It doesn't like something here... https://eicweb.phy.anl.gov/EIC/benchmarks/reconstruction_benchmarks/-/jobs/2503876
01:44:29 epic.getDete INFO Adding material from /home/runner/work/epic/epic/scripts/material_map/calibrations/materials-map.cbor
Traceback (most recent call last):
File "/home/runner/work/epic/epic/scripts/material_map/material_validation_epic.py", line 46, in <module>
detector, trackingGeometry, decorators = epic.getDetector(args.xmlFile, args.matFile)
File "/home/runner/work/epic/epic/scripts/material_map/epic.py", line 33, in getDetector
matDeco = acts.IMaterialDecorator.fromFile(
File "/usr/local/python/acts/__init__.py", line 61, in _decoratorFromFile
return ActsPythonBindings.JsonMaterialDecorator(
RuntimeError: Unable to open input JSON material file: calibrations/materials-map.cbor
./action_payload.sh: Error on line 12: ./run_material_map_validation.sh
/home/runner/work/_actions/eic/run-cvmfs-osg-eic-shell/main/setup-eic-shell.sh: Error on line 28: $THIS/run-linux.sh
Right, that's a place where I've hard-coded it ... sigh.