-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
make CPEFast to better reproduce Generic (w/o track angle) #528
base: CMSSW_11_2_X_Patatrack
Are you sure you want to change the base?
Conversation
Move duplicated Eigen code to a common file, and use it for both ECAL and HCAL. Move HCAL general reconstruction code from the hcal::multifit to the hcal::reconstruction namespace.
CachingDeviceAllocator: - add the device allocator status to the public interface - monitor the requested amount of bytes in addition to the allocated amount CUDAMonitoringService: - print CUDA memory information after each module, including stats from caching allocator
Update dependent records declarations to use edm::mpl::Vector instead of boost::mpl::vector, following cms-sw#30874.
(validating on 11_2_0_pre2) |
This comment has been minimized.
This comment has been minimized.
Seems there's some compilation error (?). Tried from scratch
And got the same issue. Don't know if I'm missing something. |
One has to start from master, not CMSSW_11_2_X_Patatrack v. |
Ugh sorry, didn't notice. Rerunning. |
I've finally synchronised For testing, right now it's probably easier to start from |
If you agree I'd directly wait for |
If you agree I'd directly wait for CMSSW_11_2_0_pre3_Patatrack. What do you think?
Ok for me.
|
This comment has been minimized.
This comment has been minimized.
Given run2_hlt_2018 conditions have been removed with 129c340ef328a6f2505facc4cc222fc2d36cd1e3 the throughput jobs on Ephemeral have been run with run3_hlt_GRun that should be the same. |
bb1d39d
to
fd98c3d
Compare
Let's re-check on top of the latest branch ... |
Please see this presentation for run3 GPU pixel only tracking studies on CMSSW_11_2_0_pre8_Patatrack using this PR which show an improvement over the standard way to do error estimation in GPUs. It also shows comparable results to the CPU. |
Validation summaryReference release CMSSW_11_2_0_pre10 at 6c149b2 Validation plots/RelValTTbar_14TeV/CMSSW_11_2_0_pre7-PU_112X_mcRun3_2021_realistic_v8-v1/GEN-SIM-DIGI-RAW
/RelValZMM_14/CMSSW_11_2_0_pre7-112X_mcRun3_2021_realistic_v8-v2/GEN-SIM-DIGI-RAW
/RelValZEE_14/CMSSW_11_2_0_pre7-112X_mcRun3_2021_realistic_v8-v1/GEN-SIM-DIGI-RAW
Validation plots (CPU vs GPU)/RelValTTbar_14TeV/CMSSW_11_2_0_pre7-PU_112X_mcRun3_2021_realistic_v8-v1/GEN-SIM-DIGI-RAW
/RelValZMM_14/CMSSW_11_2_0_pre7-112X_mcRun3_2021_realistic_v8-v2/GEN-SIM-DIGI-RAW
/RelValZEE_14/CMSSW_11_2_0_pre7-112X_mcRun3_2021_realistic_v8-v1/GEN-SIM-DIGI-RAW
Throughput plots/EphemeralHLTPhysics1/Run2018D-v1/RAW run=323775 lumi=53logs and
|
Yes, this is expected. In my opinion we need first to understand how to recover that (or at least understand the origin), cure it and eventually retune cuts at low pt |
view->m_averageGeometry = m_AverageGeometryStore.get(); | ||
view->m_cpeParams = cpeParams; | ||
view->m_hitsModuleStart = hitsModuleStart; | ||
|
||
// if empy do not bother | ||
if (0 == nHits) { | ||
if | ||
#ifndef __CUDACC__ | ||
#ifdef __cpp_if_constexpr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now that we have C++17 both on CPU and GPU, we can just use if constexpr
everywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, I think this was already commented in the integration PRs.
Appears in other places as well.
I suggest that, to better assess physics performance, this PR to be tested
|
a5f8bef
to
1049e1e
Compare
This PR improves the error estimation in CPEFast used on GPU providing a simple and fast parameterization in terms of charge, position in x and size in y.
The result is very close to what CPEGeneric provides w/o track-angle (actually a bit more accurate thanks to the estimation of the y-error using the cluster size in y (itself improved with information from the charge unbalance).
It includes also code to recompute y-error using the track-able prior to fit (off by default)
The main effect is a reduction and flattening of the chi2 and a reduction of the tails of the pulls.
Given this improvements I have (for the the being) simplified the cut in chi2 to a single value.
In my opinion further tuning of eff vs fakerate shall be implemented elsewhere (eventually in client code)
It would be also useful to test it against some Simulation with "realistic" APE (such those in the "Legacy Reprocessing")