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

chore: Remove explicit (bin0/bin1) access to MaterialSlab #3028

Conversation

asalzburger
Copy link
Contributor

This PR removes the explicit:

materialSlab(std::size_t bin0, std::size_t bin1) access from the ISurfaceMaterial base class (and the derived classes), because it only makes sense for BinnedSurfaceMaterial , and not for other surface material versions.

In all cases this was used, it was clear that you had BinnedSurfaceMaterial in hand, so you could directly access the fullMaterial() matrix.

@asalzburger asalzburger added this to the next milestone Mar 12, 2024
@github-actions github-actions bot added Component - Core Affects the Core module Component - Examples Affects the Examples module Component - Plugins Affects one or more Plugins labels Mar 12, 2024
Copy link

codecov bot commented Mar 12, 2024

Codecov Report

Attention: Patch coverage is 0% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 48.83%. Comparing base (6d1cf04) to head (2c247f5).

Files Patch % Lines
Core/src/Material/SurfaceMaterialMapper.cpp 0.00% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3028      +/-   ##
==========================================
- Coverage   48.83%   48.83%   -0.01%     
==========================================
  Files         491      491              
  Lines       28893    28889       -4     
  Branches    13705    13711       +6     
==========================================
- Hits        14111    14107       -4     
- Misses       4954     4956       +2     
+ Partials     9828     9826       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@asalzburger asalzburger requested a review from ssdetlab March 13, 2024 10:54
@asalzburger
Copy link
Contributor Author

Hi @ssdetlab - this is a tiny PR in preparation for the GridSurfaceMaterial, it removes an interface method that only makes sense for one specific implementation.

Copy link
Contributor

@ssdetlab ssdetlab left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@asalzburger, I think that for the policy-bot to be satisfied I need to be on the reviewers list. I'll send the request

@kodiakhq kodiakhq bot merged commit 4cb1e5b into acts-project:main Mar 15, 2024
54 checks passed
dimitra97 pushed a commit to dimitra97/acts that referenced this pull request Mar 19, 2024
…ct#3028)

This PR removes the explicit:

`materialSlab(std::size_t bin0, std::size_t bin1)` access from the `ISurfaceMaterial` base class (and the derived classes), because it only makes sense for `BinnedSurfaceMaterial `, and not for other surface  material versions.

In all cases this was used, it was clear that you had `BinnedSurfaceMaterial` in hand, so you could directly access the `fullMaterial()` matrix.
@paulgessinger paulgessinger modified the milestones: next, v33.1.0 Mar 26, 2024
EleniXoch pushed a commit to EleniXoch/acts that referenced this pull request May 6, 2024
…ct#3028)

This PR removes the explicit:

`materialSlab(std::size_t bin0, std::size_t bin1)` access from the `ISurfaceMaterial` base class (and the derived classes), because it only makes sense for `BinnedSurfaceMaterial `, and not for other surface  material versions.

In all cases this was used, it was clear that you had `BinnedSurfaceMaterial` in hand, so you could directly access the `fullMaterial()` matrix.
asalzburger added a commit to asalzburger/acts that referenced this pull request May 21, 2024
…ct#3028)

This PR removes the explicit:

`materialSlab(std::size_t bin0, std::size_t bin1)` access from the `ISurfaceMaterial` base class (and the derived classes), because it only makes sense for `BinnedSurfaceMaterial `, and not for other surface  material versions.

In all cases this was used, it was clear that you had `BinnedSurfaceMaterial` in hand, so you could directly access the `fullMaterial()` matrix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Core Affects the Core module Component - Examples Affects the Examples module Component - Plugins Affects one or more Plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants