RICH in DD4HEP: Issue for gas pressure, temperature conditions in Gauss and Moore
Message from @jonrob on 07-02-2025 Hi Sajan (et. al.)
Apologies for the group mail but I just wanted to start a discussion on the issue you have mentioned a couple of times now Sajan, with DD4HEP Gauss in terms of the incorrect T and P values being used by G4 to generate the RICH CK photons.
Am I correct that the crux of the issue is that the correct values, as specified in the input conditions used by the Gauss job are not being respected correctly and G4 is using some other (default?) values during generation. So then, when the reconstruction runs which does respect the conditions values, there is a mis-match ? Is that basically what the problem is ?
If that is the problem, have any steps been taken, or on going to address it. What needs to be done to fix the issue, as without a fix the RICH will never work proper in production MC.
Reply from @seaso on 07-02-2025 Dear Chris,
The crux of the issue is the following.
In ‘Detector’ when the ‘material properties’ of the gases are created, one uses the P,T values which we may label as ‘basic-default’ and the values used here are those that we had from back in the old days in the ‘mist of time’. At present, they exist in ..Detector/../compact/Rich/version-label/Rich1/DetElem/RichPropertySpecParam.xml. The ref-index tables are based on these ‘material properties’ and these tables are created in DD4HEP and then subsequently sent by DD4HEP to GEANT4. These are slightly different than the default P,T in lhcb-conditions-database and therefore in its ‘sim10/run3-ideal’ branch. They can be labelled ‘online-default’ and are in …/Conditions/Rich1(2)/online.yml.
As I understand, until recently in the new framework Gauss does not use CondDB updates and hence it ends up using the ‘basic-default’. When we run Moore from this simulated data, it picks up the ‘online-default’ which causes some problems in the shift of ‘Rec-expected’ plots, as you gather. My current ‘workaround’ is to download the lhcb-conditions-db (run3/sim10-ideal) to my area and modify the ‘online default’ to be same as ‘basic-default’. This results in the reconstructed Cherenkov angles from Moore to be slightly shifted to the right compared to those normally seen in real data. Since the upper limit of these Moore-histograms are not changed yet, these plots also appear to have a sharp cut-off at the upper end. I figured this would be the only practical solution in the short term.
I can think of a few solutions in the longer term as listed below, and I am OK with any of them.
a) Wait for Gauss to use the CondDB and ensure they are actually picked up before creating the materials in DD4HEP or at least it triggers the modification of the P,T and the creation of the corresponding ref-index tables in DD4HEP.
b) Modify the ‘basic-default’ in Detector with an MR. However, as you know this would get accepted only in the ‘trunk geometry version’ and even for this, I have no idea how long the ‘merging process’ would take.
c) I am guessing in general the ‘online-default’ is frozen, but there may be scope to modify it in the ‘sim10/run3-ideal’ branch. However, this would still need an update in Moore for the histograms. Alternately we can get Moore to use the ‘basic-default’ for simulated data, in some other way. I do not think there is any perfect solution. In this context, (b) may a good solution, but not sure if it can be made to materialize.
Reply from @jonrob on 07-02-2025 Hi Sajan,
(a) Wait for Gauss to use the CondDB and ensure they are actually picked up before creating the materials in DD4HEP or at least it triggers the modification of the P,T and the creation of the corresponding ref-index tables in DD4HEP.
For me this is the only solution that makes any sense. Even if you do (b) or (c) below you still do not guarantee the simulation and subsequent reconstruction agree on the values used, for instance if the values in the Condition DB change later on for whatever reason. (a) is the only solution where, as long as you give the Gauss and Moore steps the same condDB tag to use, they will always agree.
(b) Modify the ‘basic-default’ in Detector with an MR. However, as you know this would get accepted only in the ‘trunk geometry version’ and even for this, I have no idea how long the ‘merging process’ would take. (c) I am guessing in general the ‘online-default’ is frozen, but there may be scope to modify it in the ‘sim10/run3-ideal’ branch. However, this would still need an update in Moore for the histograms.
Alternately we can get Moore to use the ‘basic-default’ for simulated data, in some other way.
Moore (aka the reconstruction) will always use the values as specified in whatever condition DB branch/tag its been told to use. For instance
That is because I do not want the reconstruction to do anything different to what it does for real data. So this suggestion is for me a non starter.
I do not think there is any perfect solution. In this context, (b) may a good solution, but not sure if it can be made to materialize.
For me a) is the perfect (only) solution. Gauss/G4 should just respect whatever conditions versions its been configured to run using.
cheers Chris
Designs
Activity
unassigned @jonrob
So, if we wanted to implement option (a) above (which I argue is the only true solution)
Wait for Gauss to use the CondDB and ensure they are actually picked up before creating the materials in DD4HEP or at least it triggers the modification of the P,T and the creation of the corresponding ref-index tables in DD4HEPwhat technically needs to be done ?
Edited by Christopher Rob JonesJust wanted to mention that, the option (a) may require the capability to update the values and the tables in DD4HEP, before the DD4HEP sends these things to Geant4. I got the impression that once these things are created in DD4HEP-structure they are frozen, unlike the case of Detdesc where the ‘detector classes’ were part of LHCb software and thus were modifiable by us. In other words, I am not sure modifying info in a DD4HEP class after geometry closure, is foreseen in the current model. Perhaps @clemenci would know about this. (This is why I was giving (b) a better chance of success than the others; but I am OK with other options also).
Edited by Sajan Easo
-
Just to add, I am not completely against having a first quick fix, so manually synchronise the values the G4 step with DD4HEP happens to use, with those in the MC branch in the conditions DB, so what the reco step is told to use. I guess as a first step that would remove the mis-match... However, that should not be all we do here, and as I argue above the full fix is to have the G4 step properly take the CondDB values, whatever they happen to be into acount. @gcorti et. al. Would it make sense to do this as a first quick fix ?
-
@seaso I was just reading your post again above. I did not realise that the values gauss currently uses as its starting defaults are just taken from some XML files in the Detector project ?
Which files precisely are you talking about - I can find a number of different ones in Detector that seem to match
pcol ~/LHCb/stack/Master/Detector/compact/components > find . -name *SpecParam.xml | xargs ls -l -rw-r--r-- 1 jonesc UnixUsers 11146 Sep 20 16:06 ./Materials/2024-v00.00/RichRun3PropertySpecParam.xml -rw-r--r-- 1 jonesc UnixUsers 11146 Sep 20 16:06 ./Materials/trunk/RichRun3PropertySpecParam.xml -rw-r--r-- 1 jonesc UnixUsers 11146 Mar 26 2024 ./Rich/2024-v00.00/Rich1/DetElem/RichRun3PropertySpecParam.xml -rw-r--r-- 1 jonesc UnixUsers 11146 Feb 4 13:12 ./Rich/2025-v00.00/Rich1/DetElem/RichRun3PropertySpecParam.xml -rw-r--r-- 1 jonesc UnixUsers 11146 Feb 2 2024 ./Rich/before-rich1-geom-update-26052022/Rich1/DetElem/RichRun3PropertySpecParam.xml -rw-r--r-- 1 jonesc UnixUsers 11146 Feb 2 2024 ./Rich/trunk/Rich1/DetElem/RichRun3PropertySpecParam.xmlWhich of these would we need to modify to make the run3 version match what is in the condo in the MC branches ?
-
Assuming that
is the file to change if we want to affect
run3/trunkonly (?) exactly which values in there do we need to change ?Would any changes we make here only affect MC, or would anything the reco sees also get affected ?
The actual problem is that Gauss constructs Geant4 geometry during initialisation at which we do not have run number defined and so access to conditions is messy. In reconstruction, I think you are updating conditions so you can update to whatever is in conditions database. As long as in reconstruction you always overwrite default from conditions database the only effect of changing properties in Detector project should be on MC. There is work ongoing to deal with this issue in transparent way, but as this is something DD4Hep was not prepared to do it takes time.
As you noticed it is the xml file in Detector has the same name. Originally I had this file in ../Rich1/DetElem area and there was only one copy and it is the one used in 'RICH software' in Detector and elsewhere.
But, these days different copies of it exist in different geometry versions of RICH in order to comply with the requirements to have 'fixed frozen geometry versions'. In addition to this, the VP needs to create a C4F10 gas in the 'Velo-To-Rich1 region. Therefore, it seems someone copied this file to Materials area in recent months, so that they can create this in VP, without having to create the rest of RICH1.
When we run the applications, we effectivly specify the 'sub-detector' geometry version in the 'run options' file. Therefore one should specify the same 'geometry version' when running Gauss and Moore in the corresponding options file. The default that I used recently is Rich-version-v2 (ie. current 'run3/trunk') , but one can use other ones, as discussed recently the RICH software meeting.
I list below relevant the lines from the xml file in Detector and CondDB-master. Feel free to compare them.
R1:
<constant name="C4F10Temperature" value="293.*kelvin" />
<constant name="C4F10Pressure" value="1.013*bar" />
(Online.yml Pressure: 965.49918675473 Temperature: 295.0083728641 )
R2:
<constant name="CF4Temperature" value="293.*kelvin" />
<constant name="CF4Pressure" value="1.013*bar" />
(Online.yml Pressure: 964.70168675473 Temperature: 294.03354768216 )
As I understand, the Reco in Moore uses the Online.yml files, for Cherenkov angle reconstruction.
Edited by Christopher Rob Jones-
@seaso We should not change anything in online.yml in the conditions DB.
Please can you just point me to exactly which parameters, in which XML files in Detector determine the defaults used by Geat4/gauss.
Edited by Christopher Rob Jones -
@seaso Are these the parameters that control what G4/Gauss is using
<constant name="AtmospherePressure" value="1.013*bar"/> <constant name="AtmosphereTemperature" value="293.0*kelvin"/>?
If so would not simply changing these to match the values in
be the quick fix here ?
-
( @seaso please be careful with your formatting in posts - You did some weird things with
#above that caused your post to be rendered with huge fonts... ) As I understand, the Reco in Moore uses the Online.yml files, for Cherenkov angle reconstruction.
In principle the values from the xml file is essentailly available via the detector element to the reconstruction, but I think it then gets modified to the Online.yml values when the CondDB-update becomes active.
-
@seaso Why cannot you not change the defaults in the XML to better match what we actually have in reality ? i.e.
<constant name="C4F10Temperature" value="293.*kelvin" /> <constant name="C4F10Pressure" value="1.013*bar" />It seems the pressure there is the parameter that is most incorrect (in terms of what we actually have in the pit). Why not just change these ?
Yes, Indeed that is what I meant; I was trying to answer earlier question.
The option (b) I listed above is indeed this, which would change it in the simulation. It is just that is allowed only in the trunk version these days and any update in Detector as you know may also take some time to get merged. (The current default values are what somehow we had for a decade or more; but I am OK to change them).
-
just to note though, this will also affect real data as as far as I can read from the XML this will affect the Selmeir gas parameters, which the reco uses as its basis to determine its expected CK theta values. Now, changing this is probably OK on the basis we also have the scale factor corrections, which we compute for real data run by run and soak up any differences between the4se expectations and reality. It might be that changing the gasp T,P values will mean the values of these scale factors will shift, but as these are anyway arbitrary values this might be OK. we would have to be careful in checking all this though.
-
see
These scale factors are applied by the reco to any calculations it makes for the expected CK theta values, based on the gas parameters (sellmeir, T, P etc.)
For MC we have always used 1.0, as we don't need to correct for the unknown gas composition factors in real data, but there is no reason I guess we couldn't tune these...
I am not sure I like it compared to having the MC use the correct T,P values, but it might be less intrusive if changing these also affects the gas parameters for real data. Doing anything that affects real data concerns me a bit here...
-
@seaso Could you please do a test. Remove your cold DB hack to change the T,P values the reco uses and instead see what scale factors you need to fix things with dd4hep MC ?
I guess effectively it is the same thing change scale factor or default P,T. I thought if we plan to change Detector parameter, one may as well change the default to avoid confusion. But it is just an opinion. I believe the Sellmeir param is also defined in this xml file and is not changed. One can verify this in the detector element.
-
Please then open an MR that changes
<constant name="C4F10Temperature" value="293.*kelvin" /> <constant name="C4F10Pressure" value="1.013*bar" />and similar for CF4. we can then test what the impact is directly.
Edited by Christopher Rob Jones -
@seaso ping - could you please open an MR making the T and P changes so we can run some ci tests ?
-
@seaso Please do this soon. Its such a small change, just altering a few numbers it really should not take long at all. I am keen to see what impact doing this has on the reco tests..
@jonesc, As I was preparing this and testing ,I realized that the situation is a bit more complicated. Essentially there are three sources of default P,T. (a) what I have in Detector-xml. (b) CondDB-Online.yml of a default run (0?) setup by Markus, which is activated when the GasRadiatorDetector element is created (c) The CondDB-sim10-run3-ideal-branch-Online.yml. The Gauss uses (a) and I am modifying this to match to what is in (c) since this is what I use for running Gauss and Moore. However, eventually one needs to have the same in (b) and (c) also. Today I discussed this issue with @sponce and he also agrees with that. In the meantime I hope to be able to do some more tests and create the MR in the coming days.
(If you wish to be convinced of this issue, feel free to run Detector/examples/runTestRich1.sh where one can see that the T,P is printed twice, once from (a), second time from (b). Since @sponce is aware of this issue, we hope to be sort this by setting up the correct run number that the Gauss and Moore would use to access the T,P from lhcb-conditions-database for simulated data as a long term solution).
Edited by Sajan Easo-
@seaso Its a bit difficult for me to follow using the made up XML and YML file names you use above...
Can you post direct http links to the files in question you are referring to above for a) b) and c) ?
The (a) is already discussed above. These days, an example of this is in RichRun3PropertySpecParam.xml in ../Detector/compact/components/Materials/trunk area.
(b) I believe, can be found by clicking https://gitlab.cern.ch/lhcb-conddb/lhcb-conditions-database/-/tree/master/Conditions/Rich1/Online.yml?ref_type=heads and then click on the line ‘0’ online conditions flush.
(c) To see this one, one needs to go sim10/run3-ideal branch of lhcb-conditions-database indicated above and then use the same path (Conditions/Rich1/Online.yml).
-
Please just post the links, it makes life so much easier as your 'descriptions' usually take quite some decoding...
So after searching a) is just https://gitlab.cern.ch/lhcb/Detector/-/blob/master/compact/components/Materials/trunk/RichRun3PropertySpecParam.xml?ref_type=heads
For b) the 0 version is not the correct one to use - that basically means nothing valid for real data. You should look up the version for a specific run from say a representative period of last year such as what I already posted here lhcb-conddb/lhcb-conditions-database#26 (comment 9075320)
For c) it is https://gitlab.cern.ch/lhcb-conddb/lhcb-conditions-database/-/blob/sim10/run3-ideal/Conditions/Rich1/Online.yml?ref_type=heads and there unlike real data there is only a single IOV (0-inf)
b) is ground truth as it is what it was for data at that time. c) already reasonably well agrees (and if you search hard enough you can probably find a run that matches even better).
The issue is, and always has been, with a) those defaults do not match reality that well.
So in short nothing different to what I have already said above.
Edited by Christopher Rob Jones -
So @seaso to be clear. You need to update the defaults in a) to match those values in c) which the reco will use when it runs on MC using the same DB tag/branch.
-
@seaso Can you confirm which of these files need updating ?
pcol ~/LHCb/stack/Master/Detector/compact/components > find . -name *SpecParam.xml | xargs ls -l -rw-r--r-- 1 jonesc UnixUsers 11146 Sep 20 16:06 ./Materials/2024-v00.00/RichRun3PropertySpecParam.xml -rw-r--r-- 1 jonesc UnixUsers 11146 Sep 20 16:06 ./Materials/trunk/RichRun3PropertySpecParam.xml -rw-r--r-- 1 jonesc UnixUsers 11146 Mar 26 2024 ./Rich/2024-v00.00/Rich1/DetElem/RichRun3PropertySpecParam.xml -rw-r--r-- 1 jonesc UnixUsers 11146 Feb 4 13:12 ./Rich/2025-v00.00/Rich1/DetElem/RichRun3PropertySpecParam.xml -rw-r--r-- 1 jonesc UnixUsers 11146 Feb 2 2024 ./Rich/before-rich1-geom-update-26052022/Rich1/DetElem/RichRun3PropertySpecParam.xml -rw-r--r-- 1 jonesc UnixUsers 11146 Feb 2 2024 ./Rich/trunk/Rich1/DetElem/RichRun3PropertySpecParam.xmlTo be clear, we need to fix things for
run3/trunkand thus any 2025 tag made for this year, and also run4 onwards. What we do not want to do is change anything for the tagrun3/2024.Q1.2-v00.00as I suspect (to be confirmed with ci tests) doing that will invalidate all the online conditions made during data taking last year and before...run3/before-rich1-geom-update-26052022also should not be touched. The Detector-MR 682 contains the updates mentioned above. So far it seems to work OK.
(A) Regarding older versions of the xml file: Originally there was only one version of the xml file mentioned above. Considering it is also used in VP, as I understand, in recent months this got copied to ‘Materials/version’ areas and the usage of the copies in ‘RICH/version’ areas is commented out. In this context, in this MR the unused copies are removed as part of cleanup. The T,P update is made only in the version in Materials/trunk area.
(B) Regarding the Rich-v0 area (before-rich1-geom-update-26052022 area) I noticed that in ‘..compact/run3/before-rich1-geom-update-26052022’ area the ‘path.xml’ points to the intended path for RICH geometry. However, for everything else it seems to be pointing to the trunk area which is a ‘moving target’. In this context, for what it is worth, I can update the path for 'Materials' to use Materials/2024-v00.00 area in the ‘path.xml’ of this area.
Minor update for the path, done as listed above.
As for d.p. level, we all agree that such large precision is redundant. However, the current goal is to have same value here, as that in sim10/run3-ideal area. In this context, for now, I am reluctant to change this here until the corresponding values in sim10/run3-ideal area is also set to be changed. When these two areas do not have same values, it could potentially lead to having to chase up some ‘test failures’ .
mentioned in issue lhcb-conddb/lhcb-conditions-database#26
mentioned in merge request !682
mentioned in issue #74
Example plots obtained with Detector-MR 682 attached following a recent question on this in the MR