Updating branch #1
Closed
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
Adding json converter for the different track parameters types. The converter follows the pattern outlined in [nlohmann::json documentation](https://json.nlohmann.me/features/arbitrary_types/#how-can-i-use-get-for-non-default-constructiblenon-copyable-types), rather than the one present in the codebase due to the a) track parameters types not being default constructible, b) use cases connected to the grid-of-of-track-parameters conversion. To keep the converter to one implementation of the json serializer, the liberty was taken to add one more constructor to the `FreeTrackParameters`. The `GridJsonConverter` code was modified a bit to be able to handle non-default constructible types. Header for the track parameters json converter in `GridJsonConverter.hpp` is required for the grid-of-of-track-parameters conversion to be possible by the `nlohmann::json` standard. As a side note, all the types in the framework that have `to_json`/`from_json` overloaded can have grid-of-type json conversion implemented for free, if the respective headers are added to the `GridJsonConverter.hpp`. --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
This adds an option that should disable the warnings appearing in the GSF in a meaningful way... Worksaround but does not solve #3738
This is deprecated it seems.
…3758) With gcc-14, the following compilation failure is reported: ``` 438 In file included from /home/wdconinc/git/acts/Core/include/Acts/Seeding/HoughTransformUtils.hpp:374, 439 from /home/wdconinc/git/acts/Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/MuonHoughSeeder.hpp:12, 440 from /home/wdconinc/git/acts/Examples/Algorithms/TrackFinding/src/MuonHoughSeeder.cpp:9: 441 /home/wdconinc/git/acts/Core/include/Acts/Seeding/HoughTransformUtils.ipp: In member function 'std::vector<Acts::HoughTransformUtils::PeakFinders::IslandsAroundMax<identifier_t>::Maximum> Acts::HoughTransformUtils::PeakFind ers::IslandsAroundMax<identifier_t>::findPeaks(const Acts::HoughTransformUtils::HoughPlane<identifier_t>&, const Acts::HoughTransformUtils::HoughAxisRanges&)': >> 442 /home/wdconinc/git/acts/Core/include/Acts/Seeding/HoughTransformUtils.ipp:224:8: error: 'sort' is not a member of 'std'; did you mean 'qsort'? 443 224 | std::sort(candidates.begin(), candidates.end(), 444 | ^~~~ 445 | qsort >> 446 make[2]: *** [Examples/Algorithms/TrackFinding/CMakeFiles/ActsExamplesTrackFinding.dir/build.make:177: Examples/Algorithms/TrackFinding/CMakeFiles/ActsExamplesTrackFinding.dir/src/MuonHoughSeeder.cpp.o] Error 1 447 make[2]: *** Waiting for unfinished jobs.... ``` This appears to be due to a missing `#include <algorithm>` before the use of `std::sort` in the `.ipp` file. This PR adds the `algorithm` header to the `.hpp` file.
Our ROOT writers are quite scattered and I would like to group them back together.
- remove `TruthSeedRanges` as this a duplication of `ParticleSelector ` - enhance `ParticleSelector` by also input/output final particle selections which are necessary to access the number of hits - raise some pT cuts to the usual 1 GeV - handle downstream changes blocked by - #3744
The Detray material conversion was broken due to missing locality of the index map. It renames the cache object to simply `Cache` since it does more now, and through the namespace it gets clear that it is the conversion cache.
Some data seem to be not present in every root file being around...
) I noticed a few cases where the angles escape their valid range after a KF update / smoothing. We can avoid that by normalizing the angles back to their range after update. In principle other params like local position and momentum are also constrained and might need checking+fixing but for those I am not sure how to do it right now.
Adding `TrackFitterPerformanceWriter` to the track finding jobs in addition to `CKFPerformanceWriter`. blocked by - #3742
I usually expect some accumulated plots in the performance writers we can directly look at. At the same time the `CKFPerformanceWriter` is generic enough to write the performance for any finder. Therefore I propose to - Rename `TrackFinderPerformanceWriter` to `TrackFinderNTupleWriter` (this pr) - (and later) Rename `CKFPerformanceWriter` to `TrackFinderPerformanceWriter` blocked by - #3742
Followup of #3737 I usually expect some accumulated plots in the performance writers we can directly look at. At the same time the `CKFPerformanceWriter` is generic enough to write the performance for any finder. Therefore I propose to - Rename `TrackFinderPerformanceWriter` to `TrackFinderNTupleWriter` (#3737) - Rename `CKFPerformanceWriter` to `TrackFinderPerformanceWriter` (this pr) blocked by - #3742 - #3737
Cleaning up and generalizing the interface of the `PathSeeder`. Adding a concept to constrain the grid type during the instantiation.
- reset all the `ccache` caches by bumping `CCACHE_KEY_SUFFIX` up to `r2`
- rework key naming scheme
- GitHub `ccache-${{ runner.os }}-${{ github.job }}-${{ env.CCACHE_KEY_SUFFIX }}-${{ github.sha }}`
- GitLab `ccache-${CI_JOB_NAME}-${CI_COMMIT_REF_SLUG}-${CCACHE_KEY_SUFFIX}`
- GitLab: restore cache from main branch if there is none for the current branch
It seems that the current implementation is prone to symbol hiding. I worked around this by inserting another non-templated layer in the class hierarchy which can do the type check more reliably. The origin of the problem could be `-fvisibility=hidden` being toggled on all of my `ActsPythonBindings` compilation commands. It might be that this is another case where this flag leaks in via CMake from another library. I also did not want to change the visibility of the symbol manually as this would require some macros to be compiler agnostic. Some resources - https://www.qt.io/blog/quality-assurance/one-way-dynamic_cast-across-library-boundaries-can-fail-and-how-to-fix-it - https://developers.redhat.com/articles/2021/10/27/compiler-option-hidden-visibility-and-weak-symbol-walk-bar#
This adds an additional tracking efficiency plot vs `z0`. It was useful for some studies that I was doing and it may be useful also for others.
Adds central conversion between $\eta$ and $\theta$
These 2 files don't exist anymore, so we don't need them anymore in the exclude-list.
- unit tests - implement `addBoundParameters` - revisit `subtractBoundParameters`
Terminology:
- "Navigation delegate": the function that is registered with a tracking volume. In principle, this can be anything
- "Navigation policy": the object that is registered with the tracking volume. It contains a method that is connected to the "navigation delegate". It has extra methods
- "Navigation policy factory": To delay construction of the actual policy object **until after** the volume is fully sized and has all of its internal structure registered, the blueprint tree only contains *navigation policy factories*. This is configurable. During construction, the factory is applied to volumes, and produces a policy that is registered with the volume. This is called "navigation policy factory" from a conceptual point of view.
- "MultiNavigationPolicy": chains together multiple policies in a sort of composition. You can have one policy that only deals with portals, one for sensitive and one for passive surfaces, for example.
- To make this less annoying to construct, as you would have to manage the factory, I'm adding a concrete class `NavigationPolicyFactory`. Its job is to make defining a factory that produces a "MultiNavigationPolicy" easy, like:
```cpp
using namespace Acts;
using SurfaceArrayNavigationPolicy::LayerType;
SurfaceArrayNavigationPolicy::Config config{
.layerType = LayerType::Cylinder,
.bins = {10, 10}
};
auto factory = NavigationPolicyFactory::make()
.add<TryAllPortalNavigationPolicy>()
.add<SurfaceArrayNavigationPolicy>(config);
```
or in python:
```python
policy = (
acts.NavigationPolicyFactory.make()
.add(acts.TryAllPortalNavigationPolicy)
.add(acts.TryAllSurfaceNavigationPolicy)
.add(
acts.SurfaceArrayNavigationPolicy,
acts.SurfaceArrayNavigationPolicy.Config(
layerType=acts.SurfaceArrayNavigationPolicy.LayerType.Plane,
bins=(10, 10),
),
)
)
```
Part of #3502
DD4hep is currently quite log-happy. I raised the threshold in both Examples and Python a bit to avoid getting spammed.
- Reworks random bound and free track parameter generation - This should guarantee to obtain valid parameters #3756 - Split general random parameter and covariance generation - Also, don't use covariance to draw parameters in general random generation
Currently, as @krasznaa discovered, the finding of the Boost dependency in an installed version of Acts (i.e., through `ActsConfig.cmake` uses the `@ACTS_USE_SYSTEM_BOOST@` template, but this variable no longer exists. CMake, in its infinite wisdom, silently ignores this, producing `if()`, which it _again_ silently accepts and which apparently evaluates to false, so Boost is never loaded. This commit fixes the issue by ensuring that Boost is loaded unconditionally.
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Not sure why, but the URL does not work directly with a variable. related to: - #3782
- removed `Tests/UnitTests/Core/Surfaces/BoundaryToleranceTestsRefs.hpp` because it was unused. - fixed typo in the testing numbers in `Tests/UnitTests/Core/Surfaces/TrapezoidBoundsTests.cpp` - enable tests, that were planned but not finished back in the days
Sonar flagged this
Adds two free functions which allow checking `BoundVector`s and `FreeVectors`s for valid track parameters. This is then used as `assert`s in the track parameter EDM. blocked by - #3777
It looks like this is creating issues in Athena with
```console
In file included from libActsEventDict dictionary payload:146:
In file included from /cvmfs/atlas-nightlies.cern.ch/repo/sw/main--ACTS_Athena_x86_64-el9-gcc13-opt/2024-10-09T2101/Athena/25.0.20/InstallArea/x86_64-el9-gcc13-opt/include/ActsEvent/Seed.h:8:
/cvmfs/atlas-nightlies.cern.ch/repo/sw/main--ACTS_Athena_x86_64-el9-gcc13-opt/2024-10-09T2101/AthenaExternals/25.0.20/InstallArea/x86_64-el9-gcc13-opt/include/Acts/Acts/EventData/Seed.hpp:17:11: error: requires clause differs in template redeclaration
requires(N >= 3ul)
^
Forward declarations from /cvmfs/atlas-nightlies.cern.ch/repo/sw/main--ACTS_Athena_x86_64-el9-gcc13-opt/2024-10-09T2101/Athena/25.0.20/InstallArea/x86_64-el9-gcc13-opt/lib/Athena.rootmap:773:18: note: previous template declaration is here
namespace Acts { template <typename external_spacepoint_t, size_t> class Seed; }
```
/cc @krasznaa
@Ragansu and @Corentin-Allaire managed to make it work by:
> removed requires(N >= 3ul) in Seed.h and Seed.ipp and recompiled ACTS
> commented out parts in CxxUtils.h which delt with std::range when __cling__ is enabled. and recompiled Athena with a modified Package Filter.
Quoting:
> For information I think part of the issue might be that Cling doesn't like the requiere in the seed definition:
> The other issue seem to be that athena redefine the std::range::end function when Cling is used (in CxxUtils/range). Probably because it was not implemented before, but now this create a conflict with newer root version I guess ?
See: https://atlas-art-data.web.cern.ch/atlas-art-data/grid-output/main/Athena/x86_64-el9-gcc13-opt/2024-10-08T2101/InDetPhysValMonitoring/test_run4_acts_vertex_PU200/tarball_logs/payload.stdout
Co-authored-by: Pierfrancesco Butti <[email protected]>
Protect against η=`nan` in `Acts::TrackSelector::isValidTrack()`, which would `throw std::invalid_argument{"Eta is outside the abs eta bin edges"}` exception. This can come about if somehow the track θ<0 or θ>π. We should try to prevent these out-of-range values, but having them shouldn't raise an exception.
Co-authored-by: Paul Gessinger <[email protected]>
Kodiak handles this Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
I'm adding a explicit check on the input config here, because we default to an empty config, which is actually invalid, when it tries to run the selection itself. I'm also making the printing of the error code a bit more explicit in one case.
Adding some `VERBOSE` statements in the seeding, from Grid creation and Filling to seed finder and filter.
Previously, the configuration could trun through even if TBB is not present on the system.
This is a relic from codecov. We fully switched to sonarcloud.
As a step to make the gx2f easier to read, I pulled out the step, where we check out all trackstates and fill all variables of the gx2f-system. Maybe we manage in the future to remove the template using a internal track-type and put this into a separate compile unit.
The concept definition was not generic enough to cover for cases with `N != 3ul`
…3793) While troubleshooting the exception I added some logging and later removed the exception as it does not seem hurtful to double define the alias to me.
This adds simple python bindings to the test framework to convert between global and local coordinates.
I don't think this is useful and I always have to deal with this downstream
After removing `TruthSeedRanges` with #3742 we can also safely remove `TruthSeedSelector` which does very much the same as the `ParticleSelector` by now.
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.
No description provided.