GSI Helmholtzzentrum für Schwerionenforschung
Hi! After some hours of fruitless debugging I came back to the following problem: I have a script (look at recotasks/demo/geaneDemo.C) with these lines: // Setup a trackrep which uses geane CbmGeanePro* gePro=new CbmGeanePro(); TVector3 pos(0.1,0.1,0.1); TVector3 mom(0.1,0.1,1.); TVector3 poserr=TVector3(0.1,0.1,0.1); TVector3 momerr=0.2*mom; DetPlane initplane(TVector3(pos),TVector3(1.,0.,0.),TVector3(0.,1.,0.)); double q=-1; CbmTrackParP par(pos,mom,poserr,momerr,q, initplane.getO(),initplane.getU(),initplane.getV()); par.Print(); Note that px=py! These lines lead to the following output: ====================================================== Position : (0.1, 0.1, 0.1) Slopes : dx/dz = 0.0008, dy/dz = 0.0004 q/p = -0.99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Note that dx/dz!=dy/dz. Something is going really wrong! When I tried to track the problem I stumbled over the following lines. What the hack is meant with y-z-plane??? // Tranform error matrix // FROM MASTER VARIABLES (px, py,pz, x, y, z) // TO SD (transverse or local system) // VARIABLES (1/p, v', w', v, w) // // Method: the MARS system is considered as a detector system SD1 with // y-z as the detector plane. Hence eq (79) of the // report CMS 2006/001 is used to go from canonical to SD1 variables. // Then, the SD1 to SD routine is used. // In this way the track length variation and the magnetic field // effects are correctly taken into account. // // Authors: A. Rotondi and A. Fontana (July 2007) Another issue: trackbase uses the magnetic field. It get's this from the CbmRun singleton object. This construct creates nasty dependencies which make it unnecessarily hard to write tests for the code. It was always advertized that PandaRoot is getting rid of the cross-dependencies. Best Regards, Sebastian. PS: Please allow me the comment, that I am quite disappointed, that after one year this is still not in order! And that I have to dig into other people's code while at the same time apparently nobody from the geane community ever even had a look at GeaneTrackRep. It is YOUR responsibility that this works! I needed to say that. Now let's go and get the damn tracking running!
Hi, Quote: Another issue: trackbase uses the magnetic field. It get's this from the CbmRun singleton object. This construct creates nasty dependencies which make it unnecessarily hard to write tests for the code. first of all you need the field to calculate the coveriants, So if you do not get from the framework where do you get it from! The way how it is done now, enable you to get the same field which was used in simulation. Quote: It was always advertized that PandaRoot is getting rid of the cross-dependencies. This is not a cross dependency, a package in a framework depends on the base of the framework! and not the contrary. regards Mohammad
Hi Mohammad, I would like to write a small macro where I can create a track without setting up a run. This is not possible at the moment as far as I can see. In my taste this is a too close link between base and trackbase. But let's focus on the hard part: Do you have any idea what to do about the cbmTrackParP? can you reproduce this behaviour? Regards, Sebastian.
Dear Sebastian, I focus only on the technical issues that you are raising with your email, since I am certainly not in a position to tell you what your responsibilities and duties are! Our routines were carefully debugged and tested, but this does not exclude exceptions of course, which we can always fix. However we have looked at this particular case and the problem seems not to be in the code, but in the compiler's flags: the current CMakelists.txt calls a macro named Compiler_Check() that causes your error. The problem showed up after january 29th 2008, long after we released the code: we did not receive any complaint before and the same routine worked well for us (it is actually extensively used both in tutorial2 and in our Kalman filter). I do not know the purpose of this compiler macro, but it is a recent change in the svn (it was not there at the KVI workshop): I would like to ask to Mohammad what is this new makefile macro for. As a correction I think we can put a workaround by switching this flag off for geane (if this is possible). Regarding y-z, in this case it is exactly the y-z plane in MARS (you shoot along x) which is used as starting plane for the SD1 to SD2 conversion. Regards, Andrea Fontana
Hi Andrea! Thanks for your reply. I will have a look at that compiler switch. What do you say about the problem in CbmTrackParP?? I still do not understand the issue of th y-z-plane. I understand that this plane is the basis or the coordinate conversion. However this should be independent of where I shoot to, shouldn`t it? I think that the problems we war talking about are indeed on responsibilities. From my point of view it was agreed that we use GENFIT for global fitting one year ago. It was also agreed that you take over the incorporation of GEANE into GENFIT. For that purpose I sat together with you last september at GSI so I could help you start with GeaneTrackRep. This work was not completed. Since then nothing has happened on that issue. I am not familiar enough with the Geane Interface to spot problems easily. So I ask you to contribute your knowledge and help us debug the GeaneTrackRep. In my opinion the only efficient way is that you look into the code there. Best Regards, Sebastian.
Hi Andrea, Can you tell me more about the Compiler_Check() problem!! this macro check, which compiler is used on the system and set the optimization or debug flags as they where chosen by the user during configuration! So is geane sensitive to optimization flags! and how is this related to the problem in CbmTrackParP, can you please explain me more about this! Mohammad
Hi Sebastian, regarding the CbmTrackParP, we can help you and we will have a look. Regarding the division of tasks, what you say is not totally correct: what we agreed whas that our group was providing the track follower and integrating it with the interface developed by Mohammad and that your group was developing the Kalman filter structure and use Geane as a track follower. It was always intended by us that this job was to be done together: this means that we help you, by trying out your Demo class for instance. As far as I remember, in september, when we sat together, you were not helping me in integrating geane in genfit but we were helping each other, because you are not familiar with geane and we are not with genfit. I think we sent you a modified version last october where we suggested you how to change the code to be able to shoot along any direction. This did not have any follow up and we assumed it was ok for you. We can do other tests, but you are the main developer of this part. Anyway, all this discussion is not constructive: we will look into GeaneTrackRep. But if you need help on specific topics, just ask and we will help you. Regards, Andrea
Hi Mohammad, Lia has looked into this problem and it seems that the problem originates from -O2 option in the setting of the environmental variable CMAKE_CXX_FLAGS_DEBUG. We have gcc 3.4.6.Without this option the routine works correctly. We have looked into 3.4.6 manual and we found this: " Unlike most other C compilers, GCC allows you to use -g with -O. The shortcuts taken by optimized code may occasionally produce surprising results: some variables you declared may not exist at all; flow of control may briefly move where you did not expect it; some statements may not be executed because they compute constant results or their values were already at hand; some statements may execute in different places because they were moved out of loops. " In CbmTrackParP there is a call to the routine that changes track representation that seems to depend on this compilation flag. Do you have any idea? Ciao, Andrea and Lia
Hallo Andrea, Thanks for the info, I will test this and let you know! Mohammad
Hi All, So I tested the geane crash on deferent systems and compiler with and without the flags -O2 and using the code in recotasks/demo the results were: 1. Debian Sarge gcc 3.3.5 crashes with or without -O2 2. Scientific Linux 4.5 gcc 3.4.6 crashes with or without -O2 3. Fedora Core 8 gcc 4.1.2 crashes with or without -O2 4. Debian Etch gcc 4.1.2 crashes with or without -O2 5. Debian Etch 64 bit gcc 4.1.2 works with or without -O2 6. Mac OS X gcc 4.0.1 (Apple) works with or without -O2 So it seems to be independent of the flag -O2 -g and the compiler version. In all cases we will remove the -O2 from the check_compiler. I also tried to remove the whole Check_Compiler.cmake from the CMakeList.txt so that the compiler is called without options (only default) and it did not help! So we still have to understand why it works on 64 bit and not on 32!!! A question to the Pavia group: did you remove only the flag -O2 in your working environment or the whole Check_Compiler.cmake macro from the CMakelist.txt? and what system you use? regards Mohammad
Hi, After some debugging I think I found the problem or at least part of it: the crash in the recotask/demo/runDemo.C was the division by zero, and this happened when geane return a zero momentum for the propagated particle, So I added some protection against division by zero in the trackbase and trackrep classes and now it is running without any crash on all systems that I have! but still we have to control the results!. I put the corrections back in SVN, and it would be great if Sebastian and Andrea can take a look and control the results! regards Mohammad
Hi! This sounds good. I am at the moment checking in my modifications I did while traveling and I will try the demo after this. Cheers! Sebastian.
Hallo Mohammad, just a remark on the -O2 flag: it is connected only to the wrong values of the slopes Sebastian was talking about, not to the crash of the code. In fact, running just his simplified code: // Setup a trackrep which uses geane CbmGeanePro* gePro=new CbmGeanePro(); CbmTrackParP dummy; TVector3 pos(0.1,0.1,0.1); TVector3 mom(0.1,0.1,1.); TVector3 poserr=TVector3(0.1,0.1,0.1); TVector3 momerr=0.2*mom; DetPlane initplane(TVector3(pos),TVector3(1.,0.,0.),TVector3(0.,1.,0.)); double q=-1; CbmTrackParP par(pos,mom,poserr,momerr,q, initplane.getO(),initplane.getU(),initplane.getV()); par.Print(); here is what I get: 1) if I run it with the Check_Compiler and with the -O2 flag, I get the wrong values of the slopes dx/dz and dy/dz (the same values Sebastian talked about) 2) if I run it with the macro but deleting the -O2 flag in CMAKE_CXX_FLAGS_DEBUG, I get the correct values of the slopes. I am using Scientific Linux CERN SLC 4.6, gcc 3.4.6. Best regards, Lia.
Hi! Mazbe it is an unitialized variable. We had a lot of such errors recently also in other projects and the symptom with the optimization points in that direction! Sebastian.
Hi! On debian sarge gcc4.1.2 (32bit) I get the problem with the TrackParP initialization even when -O2 option is excluded. I do not see a dependence on this option. hm...
Hi! In CbmTrackParP (SD system) I found this inconsistency (at least it is an inconsistency with my understanding): TVector3 positionsd = util.FromMARSToSDCoord(TVector3(fX, fY, fZ), forigin, fiver, fjver, fkver); fU = positionsd.X(); fV = positionsd.Y(); fW = positionsd.Z(); The SD system is defined in such a way, that v and w span the detector plane. However this should correspond to x and y when I define a Detectorplane like this: DetPlane plane(TVector3(0.0,0.0,0.0),TVector3(1.,0.,0.),TVector3(0.,1.,0.)); Looking to the paper by Andrea and Pablo it is clear to me that this is indeed connected to the SP representation where the beam IS a special direction and is assumed to be the x-axis instead of the z-axis as we have in PANDA. Ideas how to solve this are most welcome! Cheers! Sebastian.
Hi! I am confused. On my debian sarge machine it seems to work. so the transformations seem to be consistent. Maybe the following is a hint: http://gcc.gnu.org/ml/gcc/2005-02/msg00053.html It could be that gcc4 does not like all the [] arrays in trackbase. Let's see. Sebastian.
Hi Sebastian, I don' t know if I understood well the problem, but first of all a remark on the SP system: the SP system is only a particular case of the SD system, which can be used when the detector planes are placed perpendicularly to the x-axis. Please note that here in the GEANE classes we never use the SP system, but only the SD (CbmTrackParP) and the SC (CbmTrackParH). Concerning the inconsistency you are talking about: is it the fact that in CbmTrackParP, where the frame is (u,v,w), the plane is spanned by v and w, while in DetPlane where the frame is (x, y, z) it is spanned by x and y? Actually I think that this would not be so dangerous if we are aware about it and treat carefully the transition from DetPlane to CbmTrackParP and vice-versa... In GEANE it has been chosen this description for the SD just to keep the old fortran GEANE one and to be able to rewrite in C++ the fortran routines (see CbmGeaneUtil) without the need to rewrite all the mathematics! Please let me know if I completely misunderstood your mail Lia.
Hi! I have found a bug in CbmTrackParP. There is an array which is uncoorrectly initalized: - Double_t PD[3], RD[6][6], H[3], CH, SP1, PC[3], RC[3]; + Double_t PD[3], RD[6][6], H[3], CH, SP1, PC[3], RC[15]; The lower row is the fix. With this the initialization problem is solved. GeaneTrackRep is running now. However I still do not get the correct results as on my old system. TBC... Cheers! Sebastian.
Hi Sebastian, I committed the fixed CbmTrackParP. Thank you for having found the bug! Lia.
Hi! Now it is running also with gcc4! At first I did not realize that there were 2 places where the RC[15] had to be corrected. But with your fix it works now. Jippie!!! Sebastian.
One more thing: During my debugging sessions I modified CbmGeaneUtil because I was suspicious about the var[] expressions. I learned that they are allowed but that you cannot use sizeof(arrayname) inside the functions (see http://de.wikibooks.org/wiki/C++-Programmierung:_Arrays german webpage - sorry for that) It will give you only the size of a pointer: memset(PD,0,sizeof(PD)); According to the information I found this is not initializing the array correctly. So probably some of the initializations will go wrong? Maybe you want to check what I did and if you like it put it to the repository? It is probably more a question of taste. The initialization issue remains an open question to me. Kind Regards, Sebastian.
Hi Sebastian, we just uploaded to svn repository your corrections to CbmGeaneUtil.cxx and .h after doing some tests. Thank you very much for your help and sorry if it took so much time ...but we finally did it! Ciao, Lia and Susanna.