iLCSoft / Marlin

Modular Analysis and Reconstruction for the LINear Collider
GNU General Public License v3.0
11 stars 16 forks source link

Commented XML strings in the StringVec parameter are wrongly parsed? #43

Closed dudarboh closed 2 years ago

dudarboh commented 3 years ago

Issue

Consider the following steering file:

<marlin>

<constants>
</constants>

<global>
    <parameter name="LCIOInputFiles">
    <!-- my_data1.slcio -->
         my_data2.slcio
    </parameter>
    <parameter name="MaxRecordNumber" value="0" />
    <parameter name="SupressCheck" value="false" />
    <parameter name="AllowToModifyEvent" value="false" />
</global>

<execute>
        <processor name="MyUsefulProcessor" />
</execute>

<processor name="MyUsefulProcessor" type="UsefulProcessor">
</processor>

</marlin>

I would expect the program to be processing only my_data2.slcio as the my_data1.slcio is commented. However running Marlin on such an steering file will read my_data1.slcio instead of my_data2.slcio!

E.g. Marlin -c ./steer.xml will output:

. . .
Loading LCIO file [my_data1.slcio]
. . .

Tested with:

iLCSoft

source /cvmfs/ilc.desy.de/sw/x86_64_gcc82_centos7/v02-02-02/init_ilcsoft.sh

dudarboh commented 2 years ago

@tmadlener @gaede

tmadlener commented 2 years ago

If this is really an XML parsing issue then it might be hard to fix inside Marlin. I will have to check if that is truly the case.

In any case if you really only want one file, there is always the possibility to pass it in via the command line, like:

Marlin --global.LCIOInputFiles=my_data2.slcio <rest of the arguments>
dudarboh commented 2 years ago

Yes, I agree that this bug is very easy to avoid, when you already know it exists :)

In my case I have a test steering file which looks something like this:

    <global>
        <parameter name="LCIOInputFiles">
            /pnfs/desy.de/ilc/prod/ilc/mc-2020/ild/dst-merged/250-SetA/4f_ZZnunu_semileptonic/ILD_l5_o1_v02/v02-02/00015147/000/rv02-02.sv02-02.mILD_l5_o1_v02.E250-SetA.I500078.P4f_zznu_sl.eL.pR.n008.d_dstm_15147_38.slcio
            <!-- /pnfs/desy.de/ilc/prod/ilc/mc-2020/ild/dst-merged/250-SetA/4f_ZZ_semileptonic/ILD_l5_o1_v02/v02-02/rv02-02.sv02-02.mILD_l5_o1_v02.E250-SetA.I500074.P4f_zz_sl.eL.pR.n000.d_dstm_15116_1.slcio -->
            <!-- /pnfs/desy.de/ilc/prod/ilc/mc-2020/ild/dst-merged/250-SetA/4f_WW_semileptonic/ILD_l5_o1_v02/v02-02/00015173/000/rv02-02.sv02-02.mILD_l5_o1_v02.E250-SetA.I500082.P4f_ww_sl.eL.pR.n001.d_dstm_15173_7.slcio -->
            <!-- /pnfs/desy.de/ilc/prod/ilc/mc-2020/ild/dst-merged/250-SetA/4f_singleZee_semileptonic/ILD_l5_o1_v02/v02-02/00015179/000/rv02-02.sv02-02.mILD_l5_o1_v02.E250-SetA.I500102.P4f_sze_sl.eL.pR.n000.d_dstm_15179_2.slcio -->
            <!-- /pnfs/desy.de/ilc/prod/ilc/mc-2020/ild/rec/250-SetA/4f_WW_hadronic/ILD_l5_o1_v02/v02-02/00015155/000/rv02-02.sv02-02.mILD_l5_o1_v02.E250-SetA.I500066.P4f_ww_h.eL.pR.n000_001.d_rec_00015155_213.slcio -->
            <!-- /pnfs/desy.de/ilc/prod/ilc/mc-2020/ild/dst-merged/250-SetA/2f_hadronic_eL_pR/ILD_l5_o1_v02/v02-02/00015161/000/rv02-02.sv02-02.mILD_l5_o1_v02.E250-SetA.I500010.P2f_z_h.eL.pR.n000.d_dstm_15161_0.slcio -->
            <!-- /pnfs/desy.de/ilc/prod/ilc/mc-2020/ild/dst-merged/250-SetA/higgs_excl/ILD_l5_o1_v02/v02-02/00015201/000/rv02-02.sv02-02.mILD_l5_o1_v02.E250-SetA.I402199.Pn23n23h_e2e2.eL.pR.n000.d_dstm_15201_1.slcio -->
            <!-- /pnfs/desy.de/ilc/prod/ilc/mc-2020/ild/rec/250-SetA/2f_hadronic_eL_pR/ILD_l5_o1_v02/v02-02/00015161/000/rv02-02.sv02-02.mILD_l5_o1_v02.E250-SetA.I500010.P2f_z_h.eL.pR.n000_052.d_rec_00015161_219.slcio -->
            <!-- /pnfs/desy.de/ilc/prod/ilc/mc-2020/ild/rec/250-SetA/2f_hadronic_eL_pR/ILD_l5_o1_v02/v02-02/00015161/000/rv02-02.sv02-02.mILD_l5_o1_v02.E250-SetA.I500010.P2f_z_h.eL.pR.n001_209.d_rec_00015161_259.slcio -->
            <!-- /pnfs/desy.de/ilc/prod/ilc/mc-2020/ild/rec/250-SetA/4f_WW_hadronic/ILD_l5_o1_v02/v02-02/00015128/000/rv02-02.sv02-02.mILD_l5_o1_v02.E250-SetA.I500068.P4f_ww_h.eR.pL.n000_350.d_rec_00015128_106.slcio -->
        </parameter>

And I always have to remember to move the un-commented file on top, because if the first line is commented, it will read the wrong file! And one, who is unaware of this issue, could spend hours (real-life story) to understand why the event doesn't look as you expect, when you test it locally and run as 1000 batch jobs which don't have any commented lines.

Also, always copy-pasting filename into a command line seems more time consuming than just one-click uncommenting the line.

If commenting in this field is e.g. unsupported, a warning/error would be nice.

dudarboh commented 2 years ago

image

[ MESSAGE "DecayChainDrawer"] -------------------------------------------------
 ***********************************************
 A runtime error occured - (uncaught exception):
      lcio::IOException: [LCReader::open()] File(s) not found:  54  event  seg  fault, need to check  
 Marlin will have to be terminated, sorry.
 ***********************************************

*** Error in `Marlin': malloc(): smallbin double linked list corrupted: 0x00000000014ec7d0 ***
======= Backtrace: =========
 . . .

Just another way I accidentally stumbled on this problem, forgetting about it.