Merge main to vertexing_group #1608
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
### Briefly, what does this PR introduce? This PR passes USE_ONNX as a compile definition and guards use of ONNX functionality with an ifdef. ### What kind of change does this PR introduce? - [x] Bug fix (issue: compilation failure when USE_ONNX=OFF) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…1508) Trying to test for and fix #1507 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Wouter Deconinck <[email protected]>
This should restore the ability to write out tracks from the central detector while using an xml configuration which doesn't contain the far-backward detectors (and vise versa) --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Dmitry Kalinkin <[email protected]>
### Briefly, what does this PR introduce? Instead of storing the mass of the proton etc in various classes, this PR adds an algorithms::ParticleSvc that distributes the correct masses and charge etc by PDG number. Note: I was playing around with using the algorithms::ParticleSvc as a thin interface-only, and let a JService read a data file and provide the actual service. But I gave up since it seemed pointless. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [x] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Dmitry Kalinkin <[email protected]>
### Briefly, what does this PR introduce? This PR introduces the postburner to EICrecon, which will remove the effects of the afterburner from the MCParticles branch, and store the resulting output (the actual MC Truth information from the generator) into a new branch, "MCParticlesHeadOnFrameNoBeamFX". The postburner does not erase any information - the user will simply have access to both the afterburned MC information via MCParticles (as before), and the new branch, which handles the correct transformations to remove the effects. The code is also staged to be able to handle removing *only* the crossing angle from reconstructed branches, and this additional functionality will come with a future PR. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [x] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ x] Tests for the changes have been added - [ x] Documentation has been added / updated - [ x] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? [pt_DVCS_protons_MCParticles_POSTBURN_ePIC_eicrecon_6_4_2024_run_0.pdf](https://github.com/user-attachments/files/15570067/pt_DVCS_protons_MCParticles_POSTBURN_ePIC_eicrecon_6_4_2024_run_0.pdf) [Analysis of ePIC Output With Afterburned Events.pdf](https://github.com/user-attachments/files/15570110/Analysis.of.ePIC.Output.With.Afterburned.Events.pdf) It only adds functionality and a new output branch, nothing else is affected. --------- Co-authored-by: Alexander Jentsch <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Dmitry Kalinkin <[email protected]> Co-authored-by: Simon Gardner <[email protected]> Co-authored-by: Wouter Deconinck <[email protected]> Co-authored-by: Sebouh Paul <[email protected]>
### Briefly, what does this PR introduce? Use Δx and Δy for determining which hits in adjacent layers are neighbors to one another in both the forward hadronic calorimeter insert and the ZDC. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [x] New feature (issue #1506 ) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [X] Tests for the changes have been added - [X] Documentation has been added / updated - [X] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? no ### Does this PR change default behavior? yes. Uses new Δx, Δy mode for the calorimeter insert and the ZDC --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Wouter Deconinck <[email protected]>
Provides forward-compatibility with PODIO 1.0
/include/podio/ROOTFrameWriter.h:7:7: note: declared here
7 | using ROOTFrameWriter [[deprecated("Will be removed in v1.0 switch
podio::ROOTWriter")]] = podio::ROOTWriter;
| ^~~~~~~~~~~~~~~
…ticleSvc (#1522) ### Briefly, what does this PR introduce? This PR should(?) fix the drich_fixed_eta reconstruction benchmark error in https://eicweb.phy.anl.gov/EIC/benchmarks/reconstruction_benchmarks/-/jobs/3441830. Unfortunately I can't reproduce the issue locally (it always works). The CI pipeline will have to tell if this works... ### What kind of change does this PR introduce? - [x] Bug fix (issue: https://eicweb.phy.anl.gov/EIC/benchmarks/reconstruction_benchmarks/-/jobs/3441830) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No.
### Briefly, what does this PR introduce? Includes clusters in the ZDC Ecal in the total energy when reconstructing neutrons ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [x] New feature (issue #1516 ) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [x] Tests for the changes have been added - [X] Documentation has been added / updated - [X] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? no ### Does this PR change default behavior? Yes. includes ZDC Ecal clusters in neutron recon. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
… Json (#1520) ### Briefly, what does this PR introduce? We do not use the Acts Identification and TGeo plugins, but we do need the Acts Json plugin. ### What kind of change does this PR introduce? - [x] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No.
…1527) Reduce pointless load on eicweb.
### Briefly, what does this PR introduce? This PR removes some unnecessary lines from reconstruction benchmark CMakeLists.txt. Without these lines, the handling of includes and libraries is centralized in `cmake/jana_plugin.cmake`. ### What kind of change does this PR introduce? - [x] Bug fix (issue: redundant code) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
### Briefly, what does this PR introduce? This sets the sampling fraction of the ZDC LYSO calorimeter back to unity, since it is a homogenous calorimeter. This will give more accurate reconstruction for low-energy photons. ### What kind of change does this PR introduce? - [x] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [x] Changes have been communicated to collaborators @sebouh137 ### Does this PR introduce breaking changes? What changes might users need to make to their code? No ### Does this PR change default behavior? Yes, for ZDC LYSO clusters --------- Co-authored-by: Sebouh Paul <[email protected]>
### Briefly, what does this PR introduce? This adapts our tracking algorithms to any Acts version between v31 and v36 (to be released, so additional refactors may hit the Acts main branch that would need porting). The commits in this branch are topologically ordered with reference to the commit message title, Acts PR number, and version validity. All commits compile individually, but no event-for-event validation has been performed. The support for mulitple versions is done by somewhat extensive use of preprocessor directives based on `Acts_VERSION_MAJOR`. Builds with: - v32.1.0: https://eicweb.phy.anl.gov/containers/eic_container/-/jobs/3481513#L3103 - v33.1.0: https://eicweb.phy.anl.gov/containers/eic_container/-/jobs/3482111#L3549 - v34.1.0: https://eicweb.phy.anl.gov/containers/eic_container/-/jobs/3482134#L3699 - v35.0.0: locally, but not yet in eic_container since this requires #1520 and related eic-spack PRs first ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [x] New feature (issue: Acts versions are released faster than we can keep up with) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No. --------- Co-authored-by: Dmitry Kalinkin <[email protected]>
This partially reverts effect of #1391. The clusterShape has become a way to extend EDM without going through extending it. We should sort those variable out into proper members, this is a first step. --------- Co-authored-by: Wouter Deconinck <[email protected]>
### Briefly, what does this PR introduce? Why not ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [x] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No ### Does this PR change default behavior? No
People were confused by us removing truth PDG in ReconstructedParticles, so we better remove this too, before it is relied upon for too much. ### Briefly, what does this PR introduce? ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No ### Does this PR change default behavior? Yes
…1536) This is to help developers to find the code using simple search.
…ents (#1533) Fixed an issue where "break" is not included in a case/break statement. ### Briefly, what does this PR introduce? When initializing the ImagingTopoCluster algorithm, there is a switch statement wherein different things are printed to inform the user what the parameters are (depending on the layer mode). This lacked breaks for the cases, causing the other print statements to be executed. This patch fixes it. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? no. no ### Does this PR change default behavior? yes. only prints out the information about the cuts for defining neighbors that is relevent based on the layer mode. --------- Co-authored-by: Dmitry Kalinkin <[email protected]>
### Briefly, what does this PR introduce? The _rMin_ parameter defined in the seeder configuration file was not being passed to the seed finder. This PR adds this. The parameter is currently set to the default value defined here: https://github.com/acts-project/acts/blob/main/Core/include/Acts/Seeding/SeedFinderOrthogonalConfig.hpp#L39 https://github.com/acts-project/acts/blob/main/Core/include/Acts/Seeding/SeedFinderConfig.hpp#L43 So, there should be no change to the output. But it will be needed in the future if we ever want to search for seeds in an outer part of the tracking detector volume. In addition, the _bFieldMin_ parameter defined in the seeder configuration file but not passed to the seed finder has been removed. This parameter does not seem to be a Acts seed finder parameter in any event. It seems to only be used if the _EstimateTrackParamsFromSeed_ class is called to fit the seed parameters (which we do not use): https://github.com/acts-project/acts/blob/main/Core/include/Acts/Seeding/EstimateTrackParamsFromSeed.hpp#L148-L149 ### What kind of change does this PR introduce? - [x] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No.
### Briefly, what does this PR introduce? Fixes: #1394 Closes: #1393 ### What kind of change does this PR introduce? - [x] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? ### Does this PR change default behavior?
### Briefly, what does this PR introduce? Actions fail on dot with `Error: trouble in init_rank`. E.g. https://github.com/eic/EICrecon/actions/runs/10234743256/job/28315055948. Since this error occurs in the EICrecon step, it may be incorrectly interpreted as an EICrecon failure. This PR splits the step in two. It is the first step in figuring out what's wrong with dot. Turns out we throw away the source and sink info that helps with rank determination. This PR also puts the source and sink back (and only removes `podio::Frame` and JEventProcessorPODIO` from them). ### What kind of change does this PR introduce? - [x] Bug fix (issue: trouble in init_rank) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No.
This should save developers from some confusing crashes. ### Briefly, what does this PR introduce? ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? ### Does this PR change default behavior?
This readout field was removed in 24.03.0. We don't need to support it. ### Briefly, what does this PR introduce? ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No ### Does this PR change default behavior? No
### Briefly, what does this PR introduce? Just a refactoring. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No ### Does this PR change default behavior? No
### Briefly, what does this PR introduce? We're at 71 warnings (https://github.com/eic/EICrecon/actions/runs/10238130891), most outside of our control in boost iostream, eigen3, and acts. This PR causes UBSan to print the actual error name to use in the suppression file, and then uses that to suppress the false positives (well, out of our scope errors). This bring us down to 34 errors. ### What kind of change does this PR introduce? - [x] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No.
### Briefly, what does this PR introduce? The JEventProcessorPODIO overrides the log level to `debug` instead of the default `info` (set by the logging service). This PR changes that behavior and the log level is now determined by the usual mechanisms. ### What kind of change does this PR introduce? - [x] Bug fix (issue: hardcoded log level is debug) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No.
…1549) ### Briefly, what does this PR introduce? Two banners is a bit much: ```console ____ _ ___ ___ _ `MM' dM. `MM\ `M' dM. MM ,MMb MMM\ M ,MMb MM d'YM. M\MM\ M d'YM. ____ MM ,P `Mb M \MM\ M ,P `Mb 6MMMMb MM d' YM. M \MM\ M d' YM. MM' `Mb MM ,P `Mb M \MM\ M ,P `Mb ,MM MM d' YM. M \MM\M d' YM. ,MM' (8) MM ,MMMMMMMMb M \MMM ,MMMMMMMMb ,M' (( ,M9 d' YM. M \MM d' YM. ,M' YMMMM9 _dM_ _dMM_M_ \M _dM_ _dMM_MMMMMMMM [INFO] Creating pipe named "/tmp/jana_status" for status info. [INFO] Setting signal handler USR1. Use to write status info to the named pipe. [INFO] Setting signal handler SIGINT (Ctrl-C). Use a single SIGINT to enter the Inspector, or multiple SIGINTs for an immediate shutdown. [INFO] Initializing... | \ \ | \ ___ \ | _ \ \ | _ \ ) | \ | ___ \ |\ | ___ \ __/ \___/ _/ _\ _| \_| _/ _\ _____| JANA2 version: 2.3.1 (unknown git status) Install prefix: /opt/software/linux-debian12-x86_64_v2/gcc-12.2.0/jana2-2.3.1-dpa4z6hu4nohqa6jw2337zvpwgw2pamp Optional deps: Podio ROOT ``` This PR removes the larger one that we print as part of EICrecon. This now ends up with e.g. ```console $ eicrecon ../../sim_dis_18x275_minQ2\=1000_craterlake.edm4hep.root [INFO] Creating pipe named "/tmp/jana_status" for status info. [INFO] Setting signal handler USR1. Use to write status info to the named pipe. [INFO] Setting signal handler SIGINT (Ctrl-C). Use a single SIGINT to enter the Inspector, or multiple SIGINTs for an immediate shutdown. [INFO] Initializing... | \ \ | \ ___ \ | _ \ \ | _ \ ) | \ | ___ \ |\ | ___ \ __/ \___/ _/ _\ _| \_| _/ _\ _____| JANA2 version: 2.3.1 (unknown git status) Install prefix: /opt/software/linux-debian12-x86_64_v2/gcc-12.2.0/jana2-2.3.1-dpa4z6hu4nohqa6jw2337zvpwgw2pamp Optional deps: Podio ROOT [INFO] Initializing plugin "/home/wdconinc/git/EICrecon/.worktree/rm-jana2-double-banner/lib/EICrecon/plugins/log.so" [INFO] Initializing plugin "/home/wdconinc/git/EICrecon/.worktree/rm-jana2-double-banner/lib/EICrecon/plugins/dd4hep.so" [INFO] Initializing plugin "/home/wdconinc/git/EICrecon/.worktree/rm-jana2-double-banner/lib/EICrecon/plugins/acts.so" [INFO] Initializing plugin "/home/wdconinc/git/EICrecon/.worktree/rm-jana2-double-banner/lib/EICrecon/plugins/algorithms_init.so" ``` ### What kind of change does this PR introduce? - [x] Bug fix (issue: double banner, all the way) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No.
…peaks need to be to split (#1554) ### Briefly, what does this PR introduce? This reduces excessive splitting observed in EEEMCal. The issue is observed by looking at E/p in log scale. The splitting causes an extended tail towards lower E/p, which reduces electron efficiency. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [x] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No ### Does this PR change default behavior? Yes
### Briefly, what does this PR introduce? The #1564 had implemented meaningful associations based on hits used. That information became available for Tracks, but end users use ReconstructedParticles, and that's what we'd like to have implemented in this PR. Closes: #1356 ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [x] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No There will be new warnings like ``` [PFRICH:RICHEndcapNLUTPID] [warning] Found a duplicate association for ReconstructedParticle at index 18 [PFRICH:RICHEndcapNLUTPID] [warning] The previous MCParticle was at 149 and the duplicate is at 200 ``` those are mostly harmless and are to be addressed separately. ### Does this PR change default behavior? Yes --------- Co-authored-by: Wouter Deconinck <[email protected]>
### Briefly, what does this PR introduce? The #1534 makes EICrecon no longer leak mass value known from truth. This changes makes it so that mass is set by PID lookup table facility according to the mass of the particle for the PDG code assigned. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No ### Does this PR change default behavior? Yes
### Briefly, what does this PR introduce? This updates the parameter which defines the _max distance in r between middle and top SP in a given seed_. This removes some minor inefficiencies for |z| > ~50mm at certain eta values, as discussed [here](https://indico.bnl.gov/event/23932/contributions/93392/attachments/55544/94996/Seed%20finding%20inefficiencies%20for%20%7Cz%7C%20%3E%2050mm.pdf). Since the beam diamond extends out to 80-100mm, this allows efficient seed finding over the whole range. Results for single negative muons generated at with p = [0.5,20]GeV/c and (vx,vy,vz) = (0,0,50mm) Before: <img width="624" alt="picture1" src="https://github.com/user-attachments/assets/404b9994-47f1-4bf1-8be5-92bb4f5f7879"> After: <img width="620" alt="picture2" src="https://github.com/user-attachments/assets/6cd720db-b346-4a29-906d-96525a3b9923"> The results look similar for particles generated at vz = 75mm. For particles generated at (vx,vy,vz) = (0,0,0), for example, there is no observed difference in the solved tracks (i.e. tracks after the ambiguity solver). The number of found seeds and unsolved tracks increases as expected. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [x] Other: Update to seed finder configuration ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [x] Changes have been communicated to collaborators @Jeet-bhai ### Does this PR introduce breaking changes? What changes might users need to make to their code? No ### Does this PR change default behavior? Yes, small improvement to tracking efficiency Co-authored-by: ShujieL <[email protected]>
### Briefly, what does this PR introduce? This PR updates the CMake projects (eicrecon should not be one, but that's for another day) so they don't search for the default C and C++ compilers, since only a C++ compiler is needed. ### What kind of change does this PR introduce? - [x] Bug fix (issue: CMake expectes CC and CXX) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No.
…arch (#1581) ### Briefly, what does this PR introduce? This PR originally intended to add hit sorting based on layer to achieve early breaks in the neighbor searching (i.e. if neighbors have to be at most two layers different in a ZDC-like detector, then in a sorted list we can stop evaluating for neighbors as soon as we encounter the first hit with more than two layers difference). This PR also makes several other changes to the topological clustering. - Hits are now referred to by an index proxy ordered set. This set (with ordering by layer and then index in collection) is modified as hits are assigned to clusters, such that only eligible hits remain in the set (once ordered, erasure does of course not affect ordering). The need for a separate vector which keeps track of already assigned hits is avoided. As more hits are assigned, the remaining search space is reduced, allowing for more efficient searching. - Clusters (groups) are now stored in a vector ordered by insertion sequence instead of a set (which is ordered but not in a useful way). This allows us to avoid rechecking already added hits for neighbors, and only check newly added hits for new neighbors to add. When modifying containers that we are iterating over, we have to make sure we do not invalidate iterators. Note https://en.cppreference.com/w/cpp/container#Iterator_invalidation. A vector container may change capacity, which invalidates all iterators. Iterators on a set are only invalidated when the element pointed to is erased. Erasure returns an iterator to the next element. Therefore, we postpone erasure of the starting hit of each group until after the entire group has been collected (and we have to make sure to avoid picking it again as a hit). If we erased the starting hit right away, the next iterator returned by the erase operation would likely become invalid when the hit is (likely, due to ordering) used in the cluster. ### What kind of change does this PR introduce? - [x] Bug fix (issue #1573) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No.
#1577) Provide a graceful deprecation for #1572 --------- Co-authored-by: Wouter Deconinck <[email protected]>
### Briefly, what does this PR introduce? This PR makes the cluster time deterministic, and not based on the whim of which hit happens to be first. It defines the cluster time as the energy-weighted average of all hit times. This is not the only deterministic choice here (earliest hit is another). ### What kind of change does this PR introduce? - [x] Bug fix (issue: cluster time depends on hit ordering) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No.
### Briefly, what does this PR introduce? Minor PR that change the labels on the capybara plots from `ref` and `rec_dis_18x275_minQ2=0_craterlake.edm4eic.root`, to `ref` and `new`. The shorter names are clearer, shorter, and don't push the legend's line type off the plotting canvas (which makes it really useless). The name of the file is in the URL as well. Now looks like:  ### What kind of change does this PR introduce? - [x] Bug fix (issue: sometimes capybara legend line type is pushed off the plotting canvas) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No.
### Briefly, what does this PR introduce? This PR fixes an issue where the `local` position of merged hits is written in DD4hep units instead of EDM4eic units. The subsequent clustering expects the `local` position to be in EDM4eic units of course. ### What kind of change does this PR introduce? - [x] Bug fix (issue: no clustering in backward HCal, https://chat.epic-eic.org/main/pl/pg6fwpimetgf8esmz4zfotwc3c) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [x] Changes have been communicated to collaborators @lkosarz ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? Fixes backward HCal clusters (should have changing number of clusters, distributed over 10x larger area).
### Briefly, what does this PR introduce? Make codespell happy. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? ### Does this PR change default behavior? Co-authored-by: Sebouh Paul <[email protected]>
Allow multisegmentation without giving a warning about it. ### Briefly, what does this PR introduce? There is an if statement in CalorimeterHitReco that checks if the segmentation type is one of several types of segmentations (cartesian or hexagonal) that prints a warning if the segmentation is not one of those. Adds MultiSegmentation to this list. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [X] Other: removal of unnecessary warning ### Please check if this PR fulfills the following: - [X] Tests for the changes have been added - [X] Documentation has been added / updated - [X] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? no ### Does this PR change default behavior? no --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Nathan Brei <[email protected]>
### Briefly, what does this PR introduce? - added association for ReconstructionParticles to output vertices ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [X] New feature (issue #1575) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [X] Tests for the changes have been added - [X] Documentation has been added / updated - [X] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? ### Does this PR change default behavior? - ReconstructedParticle association added to the default output CKFCentralVertices --------- Co-authored-by: Xin Dong <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Dmitry Kalinkin <[email protected]> Co-authored-by: Xin Dong <[email protected]>
### Briefly, what does this PR introduce? This PR adds prmon monitoring of the reconstruction jobs eicrecon-gun and eicrecon-dis, and uploads the summary as an artifact. This can be used later as a way to fail when execution time or memory usage is significantly changed by a PR. We first need to get a sense of how much variability there is due to the GitHub backend nodes. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [x] New feature (issue: catch large slowdowns in performance sooner) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No.
…hold (#1595) ### Briefly, what does this PR introduce? Uses a larger threshold for the distance between the neighboring hits in the insert, due to the new cell size. Also include "side" field (ie, left vs right) in the hit merger algorithm ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [X] Other: change of parameters ### Please check if this PR fulfills the following: - [X] Tests for the changes have been added - [X] Documentation has been added / updated - [X] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? Probably. Including the detector "side" as a field in the hit merger algorithm probably breaks the code if using a version of the epic repository from before pull request eic/epic#771 is merged. Therefore this pull request and eic/epic#771 should be merged at the same time as one another ### Does this PR change default behavior? yes. Changes the parameters of the topo clustering and also uses the "side" field in the hit merger. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Dmitry Kalinkin <[email protected]>
### Briefly, what does this PR introduce? These are already present a few lines above ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No ### Does this PR change default behavior? No
### Briefly, what does this PR introduce? This PR modifies CalorimeterHitDigi to produce raw hit associations (i.e. assocations from raw hits back to sim hits, and then from there MCParticles). The weighting is done based on total energy, and only non-trivial when merging hits. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [x] New feature (issue: raw calorimeter hit associations) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No.
github-actions
bot
added
topic: documentation
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Briefly, what does this PR introduce?
What kind of change does this PR introduce?
Please check if this PR fulfills the following:
Does this PR introduce breaking changes? What changes might users need to make to their code?
Does this PR change default behavior?