new cmake option to download the beampipe STL files from the web eos

[atolosadelgado]

BEGINRELEASENOTES

• New CMake option INSTALL_BEAMPIPE_STL_FILES can be used to download the STL (CAD model) beam pipe files from the web EOS

ENDRELEASENOTES

Introduction: the new beampipe is based on CAD models, but the STL files that contain the description are too big (36MB total) to be part of the git repository. For that reason, in the previous PR #344, the CAD files were removed.

Goal of this PR: this PR introduces an option in the CMake to download these files and place them in the proper output directory.

I was not sure if it makes sense to download and the files if the compact files are not going to be installed as well. What do you think @BrieucF @andresailer @aciarma ?

Also, maybe the cmake code is not very clean. I do not like that if the INSTALL_BEAMPIPE_STL_FILES flag is changed, one has to recompile the whole k4geo. Do you know if by creating a custom command that behavior can be prevented?

[BrieucF]

Hi Alvaro, thanks, very nice!

I agree, we should have INSTALL_BEAMPIPE_STL_FILES set to OFF as long as the CAD beampipe is not used by default.

Can you check first whether the files exist and download them only if they don't?

[atolosadelgado]

INSTALL_BEAMPIPE_STL_FILES

At the moment, if one of the STL files is not recheable, cmake gives the following message:

CMake Error at CMakeLists.txt:222 (message):
  Failed to download file: "HTTP response code said error"

Do you want to silently ignore the option for downloading in case the files do not exist? I thought it would make more sense to let it explode XD

[andresailer]

New CMake option to download the STL beampipe files from the web EOS

Mention the option in the release notes.

[BrieucF]

Can you check first whether the files exist and download them only if they don't?

Sorry I realize this might not be super clear. I meant : "if the files are already there locally in the path OUTPUTDIR/STL_FILE, then do not re-download them"

[tmadlener]

CMakes ExternalData module will handle pretty much all of this (as discussed in the meeting). The only thing left to do from our side would be to have a setup similar to e.g. in podio:

https://github.com/AIDASoft/podio/blob/b895deca9b6e37b5047de5ba44a4e867e43e722e/CMakeLists.txt#L189-L192

And then https://github.com/AIDASoft/podio/tree/master/tests/input_files (where each file contains the md5sum of the file that is referred to). Then somthing like

ExternalData_Add_Target(
  mdi_cad_files # or a more descriptive name
  SHOW_PROGRESS ON
)

For getting the files (assuming they are on eos), everytime a new file is added, we would simply need to run (on EOS)

ln -s <file> $(md5sum <file>)

[aciarma]

It seems a good solution, but then should we make sure that in each detector version we keep a master with the shape-based and a master with the CAD? This last one would work only if the INSTALL_BEAM_PIPE option is set to true of course

[atolosadelgado]

It seems a good solution, but then should we make sure that in each detector version we keep a master with the shape-based and a master with the CAD? This last one would work only if the INSTALL_BEAM_PIPE option is set to true of course

I can add a post installation step, which modifies the main XML file of the detector concept, so the beam pipe reference is changed to point to the CAD version. What do you think @BrieucF @andresailer?

[atolosadelgado]

Can you check first whether the files exist and download them only if they don't?

Sorry I realize this might not be super clear. I meant : "if the files are already there locally in the path OUTPUTDIR/STL_FILE, then do not re-download them"

I think it is a good idea, I have added the check if the STL file exists in build directory, it is not downloaded. The copy to the install directory happens always

[andresailer]

It seems a good solution, but then should we make sure that in each detector version we keep a master with the shape-based and a master with the CAD? This last one would work only if the INSTALL_BEAM_PIPE option is set to true of course

I can add a post installation step, which modifies the main XML file of the detector concept, so the beam pipe reference is changed to point to the CAD version. What do you think @BrieucF @andresailer?

Do you propose to automatically create a second detector model using the CAD MDI files if the option is set, or to change a detector model based on this flag?

[aciarma]

It seems a good solution, but then should we make sure that in each detector version we keep a master with the shape-based and a master with the CAD? This last one would work only if the INSTALL_BEAM_PIPE option is set to true of course

I can add a post installation step, which modifies the main XML file of the detector concept, so the beam pipe reference is changed to point to the CAD version. What do you think @BrieucF @andresailer?

Do you propose to automatically create a second detector model using the CAD MDI files if the option is set, or to change a detector model based on this flag?

I think it would be best to add a second detector model, not modifying the original one (at least for now)

[atolosadelgado]

It seems a good solution, but then should we make sure that in each detector version we keep a master with the shape-based and a master with the CAD? This last one would work only if the INSTALL_BEAM_PIPE option is set to true of course

I can add a post installation step, which modifies the main XML file of the detector concept, so the beam pipe reference is changed to point to the CAD version. What do you think @BrieucF @andresailer?

Do you propose to automatically create a second detector model using the CAD MDI files if the option is set, or to change a detector model based on this flag?

I think it would be best to add a second detector model, not modifying the original one (at least for now)

@andresailer Initially I was thinking about the later, but I just realized it may be bad idea if people use the k4geo from the stack.

If we do as @aciarma suggest, maybe we have to open a different PR?

[aciarma]

it can also be created from the original one, something like:

cp master.xml master_CAD.xml
sed -i 's/<include ref="path/to/shapepipe.xml"/<include ref="path/to/cadpipe.xml"/g' master_CAD.xml

[BrieucF]

As long as the CAD files are not fully valid (e.g. no overlap, no "wrong surface orientation"), I would refrain adding any master xml with them "as default". Now, automatically creating those when the flag INSTALL_BEAMPIPE_STL_FILES is set to true could indeed be a good idea

[atolosadelgado]

As long as the CAD files are not fully valid (e.g. no overlap, no "wrong surface orientation"), I would refrain adding any master xml with them "as default". Now, automatically creating those when the flag INSTALL_BEAMPIPE_STL_FILES is set to true could indeed be a good idea

so, would you like to keep this approach of using cmake to download the CAD files, or would you rather distribute a utility (a bash script for example) to do it manually? If you prefer the second, we can close this PR

[BrieucF]

I don't have a strong preference, we can keep the cmake approach if everyone agrees. I also do not think we need to automatically generate new compact files. I would propose to add a comment advertising INSTALL_BEAMPIPE_STL_FILES here and here.

[atolosadelgado]

I don't have a strong preference, we can keep the cmake approach if everyone agrees. I also do not think we need to automatically generate new compact files. I would propose to add a comment advertising INSTALL_BEAMPIPE_STL_FILES here and here.

done

[atolosadelgado]

rebased on upstream/main

[BrieucF]

The downstream build fails because of the GaudiAlg removal (nothing to do with this PR)

[BrieucF]

Can we merge this?