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

SP test for compute/ overall functionality is missing! #1036

Open
breznak opened this issue Aug 10, 2016 · 6 comments
Open

SP test for compute/ overall functionality is missing! #1036

breznak opened this issue Aug 10, 2016 · 6 comments

Comments

@breznak
Copy link
Member

breznak commented Aug 10, 2016

In a local branch I managed to break SP's functionality with all unit-tests passing!
The tests check individual methods, but we should also verify that, the most important functionality, complex compute() is correct.

I don't know if a simple, artificial test should be written, or the functionality of Hello_SP_TP , or AD in #890 can be turned into a test-case?

Desired: SP.compute() is tested

@breznak
Copy link
Member Author

breznak commented Aug 10, 2016

the test should also test output on later stages, when all the (inhibition, boosting, dutyCycles,...) could have had chance to be usedd. So eg on 1st, 100th and 10k-th iterations.

@breznak breznak mentioned this issue Aug 10, 2016
3 tasks
@rhyolight
Copy link
Member

Seems like a legit proposal to ensure that the compute method is directly covered by unit tests. I marked as under-review because I want @scottpurdy or @mrcslws to confirm. I believe sometimes core functional is tested in nupic python tests.

@breznak
Copy link
Member Author

breznak commented Aug 10, 2016

I believe sometimes core functional is tested in nupic python tests.

That sounds right, but when you are developing only for C++ ...It'd be better to have all coverage in C++ only.

@scottpurdy
Copy link
Contributor

We should fully unit test everything in the C++ with C++ tests. In this case, SpatialPooler.compute has almost no logic at all and instead delegates pretty much all of the logic to a bunch of other functions. So I'm not sure I'd write an unit tests for compute specifically.

@breznak
Copy link
Member Author

breznak commented Aug 11, 2016

@scottpurdy well, #1037 SP cleanup managed to silently corrupt that, with all unit tests passing.
(the CI fails only because I haven't removed the function from swig bindings which is removed in C++
https://travis-ci.org/numenta/nupic.core/jobs/151154581#L6679
)

And that could always happen, that is why I propose a "global functionality" test along with the "atomic functionality" unit tests.

EDIT: so well, I may not mean explicitly compute() but the "computational functionality of the SP"

@rhyolight
Copy link
Member

It is certainly not a bad idea to add a higher-level unit test around the compute function.

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

No branches or pull requests

3 participants