Skip to content
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

ref: Update to detray version 0.85.0 and adapt to new algebra-plugins API #798

Merged
merged 1 commit into from
Dec 8, 2024

Conversation

niermann999
Copy link
Contributor

@niermann999 niermann999 commented Dec 5, 2024

Test if the new algebra plugin integration works.

Edit: It turns out that algebra plugins has included the arithmetic operators for std::array vector types in the global namespace from where they were picked up in the ACTS VectorHelper by Eigen types. The operators are now moved into the algebra namespace and from there imported into the detray and traccc namspaces.

@niermann999 niermann999 force-pushed the ref-update-detray branch 3 times, most recently from dfedb51 to 2696614 Compare December 6, 2024 13:09
@niermann999 niermann999 added refactor Change the structure of the code bug Something isn't working labels Dec 6, 2024
@krasznaa krasznaa marked this pull request as draft December 7, 2024 13:04
@krasznaa
Copy link
Member

krasznaa commented Dec 7, 2024

Just to make it obvious that this is still a draft, as it uses test/development versions of algebra-plugins and detray.

@niermann999
Copy link
Contributor Author

Just to make it obvious that this is still a draft, as it uses test/development versions of algebra-plugins and detray.

The corresponding algebra-plugins tag is ready, as is the PR in detray (883)

@niermann999 niermann999 marked this pull request as ready for review December 7, 2024 14:55
Copy link
Member

@stephenswat stephenswat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy with this!

@stephenswat
Copy link
Member

But please rename the PR to something more meaningful before we merge

@niermann999 niermann999 changed the title test: algebra plugins API ref: Update to detray version 0.85.0 and adapt to new algebra-plugins API Dec 7, 2024
Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can see, traccc::size_type is only used in the I/O code. 🤔 I would much rather use std::size_t in those 2 places instead.

@niermann999 niermann999 force-pushed the ref-update-detray branch 2 times, most recently from 213fec1 to d725c17 Compare December 8, 2024 11:20
Copy link

sonarqubecloud bot commented Dec 8, 2024

Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@krasznaa krasznaa merged commit b0b5302 into acts-project:main Dec 8, 2024
27 checks passed
@beomki-yeo
Copy link
Contributor

This PR seems to slow down the benchmark?

@stephenswat
Copy link
Member

That's curious. 🤔

@niermann999
Copy link
Contributor Author

This PR seems to slow down the benchmark?

This is strange indeed, since I was just now working on the detray benchmarks and I am not seeing the same problem...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working refactor Change the structure of the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants