Open dladams opened 1 year ago
The first step here is to demonstrate it is possible to create a dataprep tool that runs a WCT algorithm. On that subject, I had some email exchanges with Brett and edited summaries follow. From me after poking into the WCT code:
I found virtual bool operator()(const boost::any& anyin, boost::any& anyout) = 0 in IFunctionNode —> IFrameFilter but I don’t see the connection to OmnibusSigProc. Does that provide this operator()?
Responses from Brett:
Link against libWireCellSigProc.so and create a concrete instance of OmnibusSigProc. On that instance, you will need to call the methods defined by IConfigurable and IFrameFilter.
and:
Call default_configuration(), alter what it returns as desired and pass to configure(). Then call operator()(const IFrame::pointer& in, IFrame::pointer& out). -Brett.
I intend to take this as a starting point.
The proper thing is to develop a WCT flow graph node that runs dataprep code.
Your current strategy will defeat DUNE's ability to benefit from multi-threading in this critical data and computationally intensive processing stage. Unless, that is, you are willing to reinvent all the MT wheels that WCT already provides.
Alternatively, DUNE can use WCT's existing Noise Filter node, instead of dataprep, which does not introduce these new problems and which has been tested on PDSP data.
Brett: Let's have that discussion in a forum outside this ticket. I would like to use this as a place to track progress toward the goal above. Briefly, we appear not to agree on the proper way to proceed and I believe the most useful place to use MT is within the tool/algorithms. Thanks.
We just had a two day workshop showing how that is not a good strategy.
It sounds like there is a problem with support for certain features in art that make a simple fcl solution to this problem not effective in terms of memory conservation. A solution that currently is clumsy but ought not to be would be to define instances of dataprep and wirecell that work only on one APA at a time. We can do this now, by restricting processing in our current chain to one APA, and replicate the trigger path entries that run dataprep and then wirecell with appropriate configurations.
The problems with this are:
1) As David says, this does not solve the memory problem -- the intermediate recob::Wire data products sent from each dataprep instance to the corresponding WireCell instance still clutter up memory. And post-WireCell producing of even newer versions of recob::Wire will consume further memory
2) Initialization of multiple instances of WireCell will take memory and CPU. And may not even work (maybe?), though most art modules are written so that they can be multiply instantiated.
Problem #1 can be solved if we can remove produced products from the event. Currently this throws an exception. Kyle has been open to removing this exception. It is meant to protect sloppy programmers, but any time there is a memory deallocation feature provided in code, it is possible to shoot ones self in the foot and we should accept that possibility. I suppose we could add an optional argument with a default value to removeCachedProduct() so that the default behavior remains for all existing code. We would then write simple deleter modules to clean up memory no longer in use. It would be good to remove cached products anyway as it provides freedom to use art's event memory for temporary data, make a plot, save it, etc.
Another feature we have asked for but not gotten is "eager writing". If we were interested in writing these cooked waveforms out to a file but not clutter memory with them we cannot use art's ROOT I/O to do that. Kyle asked at the workshop for what priority to assign to flushing data out to output files before the event processing is finished.
Another feature I am not sure really exists in art yet is module-level parallelism. The schedule parallelism in art seems tied to event processing, and communication with Kyle and others seems to indicate that if we want sub-event parallelism we'd have to use TBB ourselves.
WireCell allows us to do all of these things but we must code within it and do our own file i/o on output for example. Since WireCell is further down the link stack from dataprep and is also experiment agnostic, one would have to define plug-ins and callbacks to call the dataprep tools from within WireCell.
There are lots of benefits to using art's ROOT I/O (single output file, config info, provenance, etc). But we cannot yet stream output to it.
Tom:
I agree that we want the option to do the signal processing (raw data to ROIs at least) one APA or CRU at a time. Even if we can remove recob:Wire objects from the event data store, it does not make sense to create these objects and is preferable to pass them in the transient format used by dataprep or wirecell. I would like to be able to make multiple handoffs, e.g. noise removal in dataprep, deconvolution in wirecell and then ROI finding in dataprep. More complicated sequences including metric evaluation and event display should also be supported.
Avoiding recob::Wire conversions and event data store I/O is best accomplished by running a single module that gets input from a raw data input tool, runs a sequence of algorithms and writes the recob:Wire objects to be used as in put to hit finding. Either dataprep or wirecell can provide the module with its own algorithms. Here I am proposing to investigate enabling the former to additionally call the algorithms from latter. I will likely next work on the reverse problem: running dataprep algorithms in the wirecell module.
I expect that once we have single APA/CRU processing we will fit in 2 GB and there will no longer need the complexity of having multiple threads processing an APA and can run with a single (computational) thread. If this is not the case, then we will likely want something like TBB threads within each algorithm because processing a new APA just adds to the memory load. This then implies simultaneous processing of multiple APAs and events to try to keep the additional threads busy. It still remains interesting to offload some of the processing (e.g. FFTs) to GPUs but I expect this will be most useful within algorithms.
We are then left with difficult choice of running the dataprep module or wirecell module which has up to now avoided with ugly choice of running both. There are strong arguments for each module though naturally I prefer the first. I don't want to have that discussion here. I believe this a difficult problem that the SW architecture group should take up.
I am trying to instantiate OmibusSigProc but get this error
Consolidate compiler generated dependencies of target dunedataprep_DataPrep_Service_ThresholdNoiseRemovalService_service /home/dladams/proc/build/dev01/workdir/srcs/dunedataprep/dunedataprep/DataPrep/WctTool/test/test_wirecell.cxx:7:10: fatal error: WireCellSigProc/OmnibusSigProc.h: No such file or directory 7 | #include "WireCellSigProc/OmnibusSigProc.h" | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated.
I looked in the code for example but only found an include for the OmnibusSigProc source file:
(base) mac> grep -r OmnibusSigProc wire-cell-toolkit/ larwirecell/ | grep include wire-cell-toolkit//sigproc/src/OmnibusSigProc.cxx:#include "WireCellSigProc/OmnibusSigProc.h" (base) mac>
I presume the above error comes because dunedataprep does not have dependency on wire-cell-toolkit. I tried adding larwirecell to ups/product_deps but that alone did not help. I then additionally added this line to the top-level CMakeLists.txt:
find_ups_product( wirecell )
and instead of the above error, I saw:
[ 26%] Building CXX object dunedataprep/dunedataprep/DataPrep/WctTool/test/CMakeFiles/test_wirecell.dir/test_wirecell.cxx.o In file included from /cvmfs/larsoft.opensciencegrid.org/products/wirecell/v0_23_0/Linux64bit+3.10-2.17-e20-prof/include/WireCellAux/Logger.h:44, from /cvmfs/larsoft.opensciencegrid.org/products/wirecell/v0_23_0/Linux64bit+3.10-2.17-e20-prof/include/WireCellSigProc/OmnibusSigProc.h:4, from /home/dladams/proc/build/dev01/workdir/srcs/dunedataprep/dunedataprep/DataPrep/WctTool/test/test_wirecell.cxx:7: /cvmfs/larsoft.opensciencegrid.org/products/wirecell/v0_23_0/Linux64bit+3.10-2.17-e20-prof/include/WireCellUtil/Logging.h:14:10: fatal error: spdlog/spdlog.h: No such file or directory 14 | #include "spdlog/spdlog.h" | ^~~~~~~~~~~~~~~~~ compilation terminated.
I added spdlog in the cmake file and then got this error:
[ 26%] Building CXX object dunedataprep/dunedataprep/DataPrep/Service/CMakeFiles/dunedataprep_DataPrep_Service_InterpolatingAdcMitigationService_service.dir/InterpolatingAdcMitigationService_service.cc.o In file included from /cvmfs/larsoft.opensciencegrid.org/products/wirecell/v0_23_0/Linux64bit+3.10-2.17-e20-prof/include/WireCellUtil/Point.h:5, from /cvmfs/larsoft.opensciencegrid.org/products/wirecell/v0_23_0/Linux64bit+3.10-2.17-e20-prof/include/WireCellUtil/Pimpos.h:5, from /cvmfs/larsoft.opensciencegrid.org/products/wirecell/v0_23_0/Linux64bit+3.10-2.17-e20-prof/include/WireCellIface/IWirePlane.h:12, from /cvmfs/larsoft.opensciencegrid.org/products/wirecell/v0_23_0/Linux64bit+3.10-2.17-e20-prof/include/WireCellIface/IFrame.h:5, from /cvmfs/larsoft.opensciencegrid.org/products/wirecell/v0_23_0/Linux64bit+3.10-2.17-e20-prof/include/WireCellIface/IFrameFilter.h:6, from /cvmfs/larsoft.opensciencegrid.org/products/wirecell/v0_23_0/Linux64bit+3.10-2.17-e20-prof/include/WireCellSigProc/OmnibusSigProc.h:6, from /home/dladams/proc/build/dev01/workdir/srcs/dunedataprep/dunedataprep/DataPrep/WctTool/test/test_wirecell.cxx:7: /cvmfs/larsoft.opensciencegrid.org/products/wirecell/v0_23_0/Linux64bit+3.10-2.17-e20-prof/include/WireCellUtil/Configuration.h:4:10: fatal error: json/json.h: No such file or directory 4 | #include <json/json.h> | ^~~~~~~~~~~~~ compilation terminated.
Then I added the jsoncpp product (json didn't work) and got to this:
In file included from /cvmfs/larsoft.opensciencegrid.org/products/wirecell/v0_23_0/Linux64bit+3.10-2.17-e20-prof/include/WireCellSigProc/OmnibusSigProc.h:13, from /home/dladams/proc/build/dev01/workdir/srcs/dunedataprep/dunedataprep/DataPrep/WctTool/test/test_wirecell.cxx:7: /cvmfs/larsoft.opensciencegrid.org/products/wirecell/v0_23_0/Linux64bit+3.10-2.17-e20-prof/include/WireCellUtil/Array.h:30:10: fatal error: Eigen/Core: No such file or directory 30 | #include <Eigen/Core> | ^~~~~~~~~~~~ compilation terminated.
Adding eigen didn't fix this but when I also added this to the local cmake file:
include_directories(${EIGEN3_INCLUDE_DIR})
I did get past this and can see syntax errors in my code.
I put back the original product_deps file and get the same result:
[ 26%] Building CXX object dunedataprep/dunedataprep/DataPrep/Service/CMakeFiles/dunedataprep_DataPrep_Service_DuneDPhase3x1x1NoiseRemovalService_service.dir/DuneDPhase3x1x1NoiseRemovalService_service.cc.o /home/dladams/proc/build/dev01/workdir/srcs/dunedataprep/dunedataprep/DataPrep/WctTool/test/test_wirecell.cxx:22:7: error: ‘fhicl’ has not been declared 22 | using fhicl::ParameterSet; | ^~~~~ /home/dladams/proc/build/dev01/workdir/srcs/dunedataprep/dunedataprep/DataPrep/WctTool/test/test_wirecell.cxx: In function ‘int test_wirecell()’: /home/dladams/proc/build/dev01/workdir/srcs/dunedataprep/dunedataprep/DataPrep/WctTool/test/test_wirecell.cxx:37:3: error: ‘OmnibusSigProc’ was not declared in this scope; did you mean ‘WireCell::SigProc::OmnibusSigProc’? 37 | OmnibusSigProc osp; | ^~~~~~~~~~~~~~ | WireCell::SigProc::OmnibusSigProc In file included from /home/dladams/proc/build/dev01/workdir/srcs/dunedataprep/dunedataprep/DataPrep/WctTool/test/test_wirecell.cxx:7: /cvmfs/larsoft.opensciencegrid.org/products/wirecell/v0_23_0/Linux64bit+3.10-2.17-e20-prof/include/WireCellSigProc/OmnibusSigProc.h:22:15: note: ‘WireCell::SigProc::OmnibusSigProc’ declared here 22 | class OmnibusSigProc : public Aux::Logger, | ^~~~~~~~~~~~~~ /home/dladams/proc/build/dev01/workdir/srcs/dunedataprep/dunedataprep/DataPrep/WctTool/test/test_wirecell.cxx:40:11: error: ‘osp’ was not declared in this scope 40 | cout << osp.default_configuration() << endl; | ^~~
This is progress.
I fixed the compile-time errors and now have linker problems:
[ 28%] Linking CXX executable ../../../../slf7.x86_64.e20.prof/bin/test_wirecell /usr/bin/ld: cannot find -lWireCellSigProc collect2: error: ld returned 1 exit status
Again cribbing from larwirecell, I added this to the top-level cmake file:
cet_find_library( WIRECELL_SIGPROC_LIB NAMES WireCellSigProc PATHS ENV WIRECELL_LIB NO_DEFAULT_PATH )
That alone did not solve the problem and so I added wirecell to product_deps:
[dladams@jupyter-dladams ups]$ git diff product_deps diff --git a/ups/product_deps b/ups/product_deps index 4f9f965..9dd94f9 100644 --- a/ups/product_deps +++ b/ups/product_deps @@ -25,16 +25,17 @@ defaultqual e20 product version cetbuildtools v8_20_00 - only_for_build dunecore v09_69_01d00 +wirecell v0_23_0 end_product_list # We now define allowed qualifiers and the corresponding qualifiers for the depdencies. # Make a table by adding columns before "notes". -qualifier dunecore notes -c7:debug c7:debug -c7:prof c7:prof -e20:debug e20:debug -e20:prof e20:prof +qualifier dunecore wirecell notes +c7:debug c7:debug c7:debug +c7:prof c7:prof c7:prof +e20:debug e20:debug e20:debug +e20:prof e20:prof e20:prof end_qualifier_list # Preserve tabs and formatting in emacs and vi / vim:
I chose version v0_23_0 because I got this error when I tried the one I copied from larwirecell:
Error encountered when setting up product: wirecell ERROR: Version conflict -- dependency tree requires versions conflicting with current setup of product wirecell: version v0_7_0a vs v0_23_0 ERROR: setup -B wirecell v0_7_0a -q e20:+prof failed ERROR: setup of required products has failed
But, after all this, I still get the missing library error. I finally fixed that by changing the library reference in the local cmake file from WireCellSigProc to WIRECELL_SIGPROC_LIB. Now I get json library errors.
I see jsoncpp references I can copy from larwirecell but now I wonder if I would be better off adding explicit dependency on that package instead of wirecell in product_deps and the cmake file. Any thoughts @brettviren or @tomjunk ?
From the beginning, I have added larwirecell to product_deps, added this to top-level cmake file:
[dladams@jupyter-dladams test]$ git diff ../../../../CMakeLists.txt diff --git a/CMakeLists.txt b/CMakeLists.txt index 7978698..e90c7b3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,6 +41,11 @@ find_ups_product( larcorealg ) find_ups_product( lardata ) find_ups_product( lardataalg ) find_ups_product( lardataobj ) +find_ups_product( lawirecell ) +find_ups_product( wirecell ) +find_ups_product( jsoncpp ) +find_ups_product( spdlog ) +find_ups_product( eigen ) include(MakeDuneToolBuilder) include(dunecore::ToolTypes) # Enable simpler building of tools.
and this in WctTools/CMakeLists.txt
cet_find_library( WIRECELL_SIGPROC_LIB NAMES WireCellSigProc PATHS ENV WIRECELL_LIB NO_DEFAULT_PATH ) cet_find_library( WIRECELL_IFACE_LIB NAMES WireCellIface PATHS ENV WIRECELL_LIB NO_DEFAULT_PATH ) cet_find_library( WIRECELL_UTIL_LIB NAMES WireCellUtil PATHS ENV WIRECELL_LIB NO_DEFAULT_PATH ) cet_find_library( WIRECELL_APPS_LIB NAMES WireCellApps PATHS ENV WIRECELL_LIB NO_DEFAULT_PATH ) cet_find_library( JSONCPP_LIBS NAMES jsoncpp PATHS ENV JSONCPP_LIB NO_DEFAULT_PATH ) set(WIRECELL_LIBS ${WIRECELL_APPS_LIB} ${WIRECELL_SIGPROC_LIB} ${WIRECELL_IFACE_LIB} ${WIRECELL_UTIL_LIB} ${JSON_CPP_LIBS})
I can now compile and link and get this result from my test program:
dune-dev> workdir/build_slf7.x86_64/dunedataprep/slf7.x86_64.e20.prof/bin/test_wirecell test_wirecell: ----------------------------- test_wirecell: Instantiate OmnibusSigProc. test_wirecell: Display default config. { "ADC_mV" : 2047999999.9999998, "anode" : "AnodePlane", "break_roi_loop1_tag" : "break_roi_1st", "break_roi_loop2_tag" : "break_roi_2nd", "charge_ch_offset" : 10000, "cleanup_roi_tag" : "cleanup_roi", "ctoffset" : -8000, "decon_charge_tag" : "decon_charge", "dft" : "FftwDFT", "elecresponse" : "ColdElec", "extend_roi_tag" : "extend_roi", "fft_flag" : 0, "field_response" : "FieldResponse", "frame_tag" : "sigproc", "ftoffset" : 0, "gain" : 2.2430470818000003e-12, "gauss_tag" : "gauss", "inter_gain" : 1.2, "isWarped" : false, "loose_lf_tag" : "loose_lf", "lroi_jump_one_bin" : 0, "lroi_max_th" : 10000, "lroi_rebin" : 6, "lroi_short_length" : 3, "lroi_th_factor" : 3.5, "lroi_th_factor1" : 0.69999999999999996, "mp2_roi_tag" : "mp2_roi", "mp3_roi_tag" : "mp3_roi", "per_chan_resp" : "PerChannelResponse", "r_break_roi_loop" : 2, "r_fake_signal_high_th" : 1000, "r_fake_signal_high_th_ind_factor" : 1, "r_fake_signal_low_th" : 500, "r_fake_signal_low_th_ind_factor" : 1, "r_low_peak_sep_threshold_pre" : 1200, "r_max_npeaks" : 200, "r_pad" : 5, "r_sep_peak" : 6, "r_sigma" : 2, "r_th_factor" : 3, "r_th_peak" : 3, "r_th_precent" : 0.10000000000000001, "rebase_nbins" : 200, "shaping" : 2200, "shrink_roi_tag" : "shrink_roi", "sparse" : false, "tight_lf_tag" : "tight_lf", "troi_asy" : 0.10000000149011612, "troi_col_th_factor" : 5, "troi_ind_th_factor" : 3, "troi_pad" : 5, "use_multi_plane_protection" : false, "use_roi_debug_mode" : false, "use_roi_refinement" : true, "verbose" : 0, "wiener_tag" : "wiener", "wiener_threshold_tag" : "threshold" } test_wirecell: ----------------------------- test_wirecell: Done.
I pushed my changes to develop. I first had to pull recent changes including this odd package tag: v09_69_01g01 which I left in produt_deps.
Poling around in wirecell toolkit, it looks like SimpleFrame is the concrete IFrame subclass that I should use to pass data to and retrieve data from wirecell. Later I might look at making my own class with the IFrame interface wrapped around dataprep structures.
I am creating a utility DpWctFrameConverter that is constructed from an AdcChannelDataMap and provides methods to extract or set its waveforms to or from an IFrame object.
@brettviren I see that IFrame holds a interger identifier "ident". Is that simply geometry identifier or does it indicate the stage of processing? Most important, when we apply a transformation, will the output IFrame have the ident as the input? Thanks. --da
After some time off for school breaks, I am back working on the dataprep-wirecell frame converter. I at least want to get it to build so I can push the code to develop. After updating header and source, I can nowI can now compile. To get things to link, I added many missing inline directives in AdcChannelData.h and added find_library directives for the wirecell aux library.
The current DUNE reco strategy runs a dataprep module which writes full (i.e. no ROI filtering) processed waveforms as recob:Wire to the event data store which are used as input to the WCT (wirecell) module which does deconvolution and ROI finding (and more?) writing another recob::Wire container to be used in hit finding. There are two problems with this approach:
Point 2 could be addressed by running another dataprep module after WCT without ROI finding but that implies yet another recob::Wire entry in the event data store and even another if we utlimately decide to then run another WCT module to do the ROI finding. A second approach would be to somehow insert dataprep tools in the WCT processing chain but this looks difficult as WCT has a private notion of tools and configuration. The subject of this issue is a third approach where WCT sequences are wrapped as dataprep tools so they can be called in the dataprep processing chain. If successful, it will then be possible to run the full signal processing chain (raw data to ROIs suitable for hit finding ) in a dataprep module with options to take algorithms for decovolution and ROI algorithms from WCT or from other sources.