search

LArSoft / larwirecell

This code is part of the Liquid Argon Software (LArSoft) project. It contains simulation and reconstruction algorithms for LAr TPC detectors. If you have a problem, please log a redmine issue: https://cdcvs.fnal.gov/redmine/projects/larsoft/issues/new
0 stars 18 forks source link

LArWireCell Update to enable multiple signal response simulation across the YZ Plane #44

Open jzennamo opened 8 months ago

jzennamo commented 8 months ago

This pull request enables the creation of SimChannels for a DepoSet that might only be tied to a single plane. If a specific set of Depos only fall onto one plane these changes enable the creation of SimChannels only for that plane.

This will result in a large number of SimChannel collections (which was already the case) a future PR could address this by adding in a merging of these collections.

FNALbuild commented 8 months ago

A new Pull Request was created by @jzennamo for develop.

It involves the following packages:

larwirecell

@LArSoft/level-1-managers, @LArSoft/level-2-managers can you please review it and eventually sign? Thanks.

cms-bot commands are listed here

FNALbuild commented 8 months ago

The code-checks are being triggered in jenkins.

FNALbuild commented 8 months ago

-code-checks Pull request failed code-formatting checks. Please ensure that cetmodules has been setup and execute the following command from the top-level directory of your repository:

format-code \
  larwirecell/Components/DepoSetSimChannelSink.cxx \ 
  larwirecell/Components/DepoSetSimChannelSink.h \ 
  larwirecell/Components/SimChannelSink.cxx \ 
  larwirecell/Components/SimChannelSink.h

Then commit the changes and push them to your PR branch.

FNALbuild commented 8 months ago

Pull request #44 was updated. @LArSoft/level-1-managers, @LArSoft/level-2-managers can you please check and sign again.

FNALbuild commented 8 months ago

The code-checks are being triggered in jenkins.

FNALbuild commented 8 months ago

+code-checks

HaiwangYu commented 8 months ago

Thanks for making this PR. We are working on it to make sure nothing will be broken.

brettviren commented 8 months ago

@jzennamo Thanks for this. My only comment is the two components you update are "deprecated". The new DepoFluxWriter is the one we should use to form SimChannel::IDE from now on. Would you like to add your process_planes feature there?

knoepfel commented 7 months ago

@jzennamo, are you willing to move your changes to the DepoFluxWriter component?

knoepfel commented 7 months ago

@jzennamo, are you willing to move your changes to the DepoFluxWriter component?

jzennamo commented 7 months ago

Yes, I can make this fix, sorry for my delayed response

jzennamo commented 7 months ago

@brettviren I don't see where DepoFluxWriter lives in larwirecell, could you point me in the right direction?

brettviren commented 7 months ago

@jzennamo

https://github.com/LArSoft/larwirecell/blob/master/larwirecell/Components/DepoFluxWriter.cxx

It looks like you started this PR branch prior to that development. You can merge master or develop to pick up the subsequent additions. You'll probably want to do that merge in any case to smooth the eventual merging back in of this PR branch.

knoepfel commented 6 months ago

@jzennamo, is there any update on this PR?

knoepfel commented 4 weeks ago

@jzennamo, this PR has been open for a while. Shall we keep it open? Or do you intend to open a separate PR once you adopt the suggestion from Brett?