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

Improve DynRankView debug check #541

Closed
ndellingwood opened this issue Nov 14, 2016 · 2 comments
Closed

Improve DynRankView debug check #541

ndellingwood opened this issue Nov 14, 2016 · 2 comments
Assignees
Labels
Bug Broken / incorrect code; it could be Kokkos' responsibility, or others’ (e.g., Trilinos) Enhancement Improve existing capability; will potentially require voting
Milestone

Comments

@ndellingwood
Copy link
Contributor

ndellingwood commented Nov 14, 2016

The verify_dynrankview_rank( N , drv ) routine used during debugging (specifically when bounds check is enabled) currently checks that the rank of the parenthesis operator ( the 'N' value above ) is greater than or equal to the dynamic rank, and aborts otherwise. This check, rather than equality, was chosen to allow for extra zero arguments to be passed to the DynRankView, which was needed for Intrepid2 refactoring and generic functors for various tensor/contraction operations if I remember correctly.

With FAD types that are not created through the Sacado factory this can result in a situation where the results of this check pass but would be expected to fail.
@rppawlo @kyungjoo-kim @etphipp

@etphipp
Copy link
Contributor

etphipp commented Nov 14, 2016

Would it be reasonable to check the extra indices are in fact zero? I think that would catch the errors Roger was running into.

@ndellingwood ndellingwood self-assigned this Nov 15, 2016
@ndellingwood
Copy link
Contributor Author

I added checks like @etphipp, tested on my laptop. Will run more rigorous tests and after passing will issue a pull request.

ndellingwood added a commit to ndellingwood/kokkos that referenced this issue Nov 15, 2016
This commit addresses Kokkos Github issue kokkos#541

If more arguments than the dynrankrank are passed to the parenthesis
operator of a dynamic rank view, check that these are zero. This is
allowed, but any non-zero value corresponds to an invalid address
access, will abort with a message to user.
ndellingwood added a commit to ndellingwood/kokkos that referenced this issue Nov 28, 2016
This commit addresses Kokkos Github issue kokkos#541

Enhancements made to debug bounds checking macros and routines to verify
that arguments passed to an operator beyond the DynRankView's rank are 0

	modified:   src/Kokkos_DynRankView.hpp
ndellingwood added a commit to ndellingwood/kokkos that referenced this issue Nov 29, 2016
This commit addresses Kokkos Github issue kokkos#541

Enhancements made to debug bounds checking macros and routines to verify
that arguments passed to an operator beyond the DynRankView's rank are 0

	modified:   src/Kokkos_DynRankView.hpp
@crtrott crtrott added Bug Broken / incorrect code; it could be Kokkos' responsibility, or others’ (e.g., Trilinos) bug - fix pushed to develop branch Enhancement Improve existing capability; will potentially require voting labels Dec 1, 2016
@crtrott crtrott added this to the END 2016 milestone Dec 1, 2016
@crtrott crtrott closed this as completed Dec 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Broken / incorrect code; it could be Kokkos' responsibility, or others’ (e.g., Trilinos) Enhancement Improve existing capability; will potentially require voting
Projects
None yet
Development

No branches or pull requests

3 participants