-
Notifications
You must be signed in to change notification settings - Fork 279
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
Fix failed test SimpleTwoRegionNetworkIntrospection on 32 bit Linux #1438
base: master
Are you sure you want to change the base?
Conversation
@lscheinkman do you mind reviewing this when you have time? |
@paulscode Have you never signed the contributor license? |
I signed it yesterday. Do I need to do anything else to make the validator pick it up? |
It should pick it up now. If not, ignore the validation failure I have confirmed you've signed it. |
Nevermind Paul, those windows build errors were there already. Luiz is planning on working on it but not sure exactly when. |
Resolved the standards-related error reported by bamboo |
@paulscode can you merge in master as soon as #1442 is merged? Then CI will run again with his changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paulscode Could you add a test for the new Dimensions(std::vector<unsigned long> v)
constructor?
See https://github.com/numenta/nupic.core/blob/master/src/test/unit/ntypes/DimensionsTest.cpp#L176
@lscheinkman, doesn't the testSimpleTwoRegionNetworkIntrospection test cover this constructor? Or you looking for something specific for only testing the constructor by itself? |
@paulscode You are correct, |
@paulscode I think this is waiting for one more C++ test. |
Oh, right. Thanks for the reminder. |
This solves issue #1437. New to swig, so may not be the best solution. Should at least highlight the source of the failure. Please share any ideas for solving the problem differently.