Add sanity checks for predicates on vectors #702
Conversation
stephenswat
added
feature
stephenswat
assigned niermann999 and beomki-yeo and unassigned niermann999 and beomki-yeo
stephenswat
force-pushed
the
feat/sanity_predicate
branch
3 times, most recently
from
2d600b7
to
9ff9e07
Compare
stephenswat
force-pushed
the
feat/sanity_predicate
branch
from
9ff9e07
to
ac8ea7c
Compare
stephenswat
force-pushed
the
feat/sanity_predicate
branch
from
ac8ea7c
to
9470cf9
Compare
|
Feedback incorporated; thanks @niermann999. |
stephenswat
force-pushed
the
feat/sanity_predicate
branch
from
9470cf9
to
302516c
Compare
|
It looks harmless but could you explain the motivation. |
I want to check whether the volumes in the parameters in the CKF actually make sense, because I am running into a bug where we are navigating through invalid navigation volumes. |
stephenswat
force-pushed
the
feat/sanity_predicate
branch
from
302516c
to
d8a491f
Compare
This commit adds four new sanity checks, `true_for_all`, `false_for_all`, `true_for_any`, and `false_for_any` which basically do what they say on the tin.
There was a problem hiding this comment.
In general I understand the motivation I believe, but I'm not fond of adding generic code in a project without a concrete use case being there already. I.e. I would prefer seeing these predicates in action as well. Otherwise the design might need to be modified significantly once we actually want to use this code. 🤔
But more than that, check if you could implement the code such that it would be less AoS specific. See 28e8247, 7fe10dc and 9641666 to see what I had to do with the existing code to make it work with SoA containers.
This commit adds four new sanity checks,
true_for_all,false_for_all,true_for_any, andfalse_for_anywhich basically do what they say on the tin.