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

test_AD "normal" kernels vs MOKernels #416

Open
st-- opened this issue Dec 18, 2021 · 3 comments
Open

test_AD "normal" kernels vs MOKernels #416

st-- opened this issue Dec 18, 2021 · 3 comments

Comments

@st--
Copy link
Member

st-- commented Dec 18, 2021

#263 by @david-vicente introduced separate methods for test_AD for MOKernel - I'm concerned this means that we won't be running the same set of tests for MOKernels.

Maybe that's how it has to be, but then it seems like the type hierarchy isn't quite right - we should obey Liskov substitution principle, right?

This to me seems like a case where overloading the same method isn't really the right thing to do (because it suggests they do the same thing - it's a kernel AD check in either case - but they don't actually seem to check the same things)... now that i've got y'all in a thread, what is your opinion on these ?

Originally posted by @st-- in #414 (comment)

@willtebbutt
Copy link
Member

Hmm yeah, I think you're right -- this slipped by us. Presumably we should just be constructing MOKernel-specific inputs (in the default case) and passing these on to the regular testing function or something (i.e. like we do with the interface tests). Is this the kind of thing that you had in mind?

@david-vicente
Copy link
Contributor

Sorry for the trouble. At the time I was having a hard time using the existing methods because I wanted to perform AD tests for a kernel with tuples of Ints and Floats as inputs, and the existing code didn't consider that case.

@willtebbutt
Copy link
Member

Ahhh yes, now I remember. The Julia AD testing infrastructure has progressed since we last looked at this, so maybe it's able to handle it now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants