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

Stokhos(mask logic): MaskLogic::OP should take by const& #12325

Open
romintomasetti opened this issue Sep 25, 2023 · 5 comments
Open

Stokhos(mask logic): MaskLogic::OP should take by const& #12325

romintomasetti opened this issue Sep 25, 2023 · 5 comments
Labels
pkg: Stokhos type: enhancement Issue is an enhancement, not a bug

Comments

@romintomasetti
Copy link
Contributor

Enhancement

@trilinos/stokhos

@etphipp

When we run tests using ensembles, we want to make floating-point equality checks (or at least comparison within a tolerance).

Making such thing sample-wise is ensured by using MaskLogic::OP.

I think these MaskLogic::OP should take Mask<T> by const&. Otherwise, it copies Mask<T> uselessly.

namespace MaskLogic{
template<typename T> KOKKOS_INLINE_FUNCTION bool OR(Mask<T> m){
return (((double) m)!=0.);
}
KOKKOS_INLINE_FUNCTION bool OR(bool m){
return m;
}
template<typename T> KOKKOS_INLINE_FUNCTION bool XOR(Mask<T> m){
return (((double) m)==1./m.getSize());
}
KOKKOS_INLINE_FUNCTION bool XOR(bool m){
return m;
}
template<typename T> KOKKOS_INLINE_FUNCTION bool AND(Mask<T> m){
return (((double) m)==1.);
}
KOKKOS_INLINE_FUNCTION bool AND(bool m){
return m;
}
}

If you agree, I'll make a PR to fix this.

@romintomasetti romintomasetti added the type: enhancement Issue is an enhancement, not a bug label Sep 25, 2023
@romintomasetti
Copy link
Contributor Author

@maartenarnst will also be interested 😉

@etphipp
Copy link
Contributor

etphipp commented Sep 25, 2023

The compiler often eliminates the copy, but const& is better. A PR would be great, thanks!

@romintomasetti
Copy link
Contributor Author

romintomasetti pushed a commit to uliegecsm/Trilinos that referenced this issue Oct 3, 2023
romintomasetti pushed a commit to uliegecsm/Trilinos that referenced this issue Jan 30, 2024
Copy link

github-actions bot commented Oct 6, 2024

This issue has had no activity for 365 days and is marked for closure. It will be closed after an additional 30 days of inactivity.
If you would like to keep this issue open please add a comment and/or remove the MARKED_FOR_CLOSURE label.
If this issue should be kept open even with no activity beyond the time limits you can add the label DO_NOT_AUTOCLOSE.
If it is ok for this issue to be closed, feel free to go ahead and close it. Please do not add any comments or change any labels or otherwise touch this issue unless your intention is to reset the inactivity counter for an additional year.

@github-actions github-actions bot added the MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. label Oct 6, 2024
@etphipp etphipp removed the MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. label Oct 6, 2024
@etphipp
Copy link
Contributor

etphipp commented Oct 6, 2024

I’m retesting the linked PR to see what the status is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: Stokhos type: enhancement Issue is an enhancement, not a bug
Projects
None yet
Development

No branches or pull requests

3 participants