SNDSW - FEDRA conversion functions#132
SNDSW - FEDRA conversion functions#132antonioiuliano2 wants to merge 21 commits intoSND-LHC:masterfrom
Conversation
antonioiuliano2
force-pushed
the
ConvertEmuBaseTracks
branch
from
0c11432 to
300acee
Compare
|
Dear all, with the help of @dcentanni , I have updated the pull request with the automated script to perform all digitization and reconstruction. I have tested it with one of the test simulations produced by Martina (the 10 GeV one). |
|
Good morning, Is there any comment or suggestion for this pull request? If not, can we merge it so everyone can use these functions? Best Regards, |
olantwin
left a comment
olantwin
left a comment
There was a problem hiding this comment.
I can't judge the FEDRA side of things, but it looks like this should not interfere with any existing sndsw code.
antonioiuliano2
force-pushed
the
ConvertEmuBaseTracks
branch
from
d413242 to
5aee9dc
Compare
| TGeoNavigator* nav = gGeoManager->GetCurrentNavigator(); | ||
| //going there | ||
| nav->cd(pathtoplate.Data()); | ||
| //returning position to master |
There was a problem hiding this comment.
My bad sorry, it should be angles, or even better vector.
| //from sndsw cm to emulsion micron and from center to corner (our scanning takes 0,0 in a corner) | ||
| } | ||
|
|
||
| void EmulsionDet::GetLocalAngles(Int_t id, const Double_t* globalang, Double_t* localang){ |
There was a problem hiding this comment.
I think these thin wrappers around the transformation methods of the TGeoNavigator adds very little.
It might be worth just getting this navigator once and then just using its methods directly.
There was a problem hiding this comment.
I was following what is done in the GetPosition functions for the electronic detectors, using the navigator to access the position of the hit. What would you propose to do instead?
There was a problem hiding this comment.
Where are these actually used?
If this mirrors the naming conventions and interface for the other detectors, you should follow them, but in my opinion the naming is very confusing and the abstraction offers little benefit for a lot of boilerplate...
| @@ -0,0 +1,40 @@ | |||
| #!/bin/bash | |||
|
|
|||
| # Bash script wrapping all of "FairShip2Fedra" conversion tools | |||
There was a problem hiding this comment.
Why not give this scripts some arguments so that one doesn't have to move files around?
This would allow using it e.g. in HTCondor without too many acrobatics.
There was a problem hiding this comment.
I wonder whether e.g. a makefile might be more appropriate to steer the conversion process.
olantwin
left a comment
olantwin
left a comment
There was a problem hiding this comment.
Seems like the changes should not affect any other parts of sndsw. I've added some comments on the individual scripts.
For the new C++ and python files, I would recommend formatting them using clang-format and black respectively, as some of them are not easy to read.
For the shell scripts, it might be worth running shellcheck over them, as they look quite fragile. Maybe add some error checking as well?
There was a problem hiding this comment.
will these changes have any impact on existing MC data being read with the new code?
There was a problem hiding this comment.
No, it will only provide additional information. It fixes a bug in the DecodeBrickID, and it provide passage of position from local to global coordinates and viceversa.
All the information is retrieved with the TGeoNavigator from the geofile, so it should reflect the geometry of the MC simulation (if the right geofile is provided)
There was a problem hiding this comment.
What is the use case for drawEmuTrack?
For the trident matching, I take the vertex and the attached track slopes from the emulsion scan, and overlay the electronic data with a given emu vertex. Only requires to read an ascii file once. I am wondering how much functionality should be inside the event display, if it would not be better to provide some already converted data to the event display.
In the method below, initialization is called for every call of the draw method. Is this on purpose? There is drawEmuTrack with argument itrack and brickID, and there is also drawEmuVertex with same arguments. Wouldn't it be better to combine both? Otherwise, the user has to take care to call both with the same arguments.
There was a problem hiding this comment.
I agree, I basically used this method for testing the code when I was developing, since displaying tracks and vertexing was the easiest way to check that the positions were in the right place;
At production, a more efficient way can be surely pursued, I agree we can put the function in a different python script and then pass the converted data to the event display.
|
The conversion from the emulsion coordinate system (assuming cm) as taken from the code (geometry/sndLHC_TI18geom_config.py) is reported to be DX = 46.8 and DY = -15.5. The conversion in question produces the following distance between the true MC shower center (determined by the distance to the center of the bin with the largest content with bin widths < 0.5mm) and the segments over 100 events of pi0 with energy between 100 MeV and 5 TeV spread out over the entire emulsion detector:
An attempt was made to change the distance manually to DX = 47.3; DY = -15.8; (trial and error was used to ensure the distance peak was as close as possible to 0, high confidence that the results are accurate within 1mm) This improves the performance and produce the following distance of segments and tracks respectively (for 100-1000 events of e- this time, same geometry):
The distance peak evidently cannot be brought to 0 as radial expansion is being looked at here. All of the displacements examples above were hard coded into the analysis code and are not part of SNDSW software. |
|
Hello @matclim, Thank you for your feedback! Could you please add the information about which Monte Carlo production are you using? Concerning this old pull request, I believe we should isolate the new conversion functions directly related to the SNDSW classes, and make a cleaner and updated pull request. All the other Monte Carlo and FEDRA scripts are very old and now outdated, since the working versions are already included in the Emulsion Reconstruction repository. |
|
The MC script that was used was before spread production was implemented but was as follows: python $SNDSW_ROOT/shipLHC/run_simSND.py -n 100 --PG --pID 111 --Estart 0.01 --Eend 5000 --EmuDet the latest commit from the master is 15f50a8 not other geometry changes were made. Some changes were added in shipLHC/run_simSND.py to the primGen in order to allow for the diffuse production of particles all over the target. Concerning the last point, should a new issue be created? |
5 participants
Dear All,
With this pull request I am organizing the functions I use to move between the detector reference system using for the scanning (units in micron, y vertical, each brick coordinates from 0 to 192000), and the physics reference system used in SNDLHC global data analysis (units in cm, slight rotation, unique origin for all the system). The transformation is done in the class EmulsionDet, by using LocalToMaster for positions and LocalToMasterVect for angle.
Currently, the passage from and to FEDRA objects is done in a separate python module, but it may also be added to the EmulsionDet class if we would prefer to have all in C++.
I have also added the drawing function to the 2D event display.
I attach here an example of a simulated muon neutrino vertex display, both in our FEDRA system and in the SNDSW system.
Please let me know if you have any comment or question,
Antonio