SP smarter params WIP #279
Conversation
- uses same codepath for constructor and set* methods - more strict params checks
SP behaves differently when constructor vs setters are used to set the same parameters!
|
I'm getting a segfault in Debug on CppRegionImpl_ fanIn, @dkeeney you've been working with the code, could you have a look if you have time, please?
|
| potentialRadius_ = potentialRadius; | ||
| NTA_CHECK((UInt)(potentialPct_ * potentialRadius_) >= 1u) << "SP: at least 1 input synapse must be able to activate from potential pool."; | ||
| NTA_CHECK(stimulusThreshold_ <= potentialPct_ * potentialRadius) << "Stimulus threshold must be <= than the number of possibly active input synapses per segment."; |
There was a problem hiding this comment.
potentialPct_ * potentialRadius is not the size of the potential pool, in the case where there are multiple dimensions. For this you can query the connections class?
There was a problem hiding this comment.
is not the size of the potential pool, in the case where there are multiple dimensions
This is from the time when SP was essentially 1D and any nD inputs/SP were simulated by converting to 1D columnar array.
I was going to suggest changing Real potentialRadius to vector<Real> potentialRadii where size potentialRadii == num dimensions of inputs
For this you can query the connections class?
no comprende. afaik the Connections doesn't have a concept of potential pools(?) Do you suggest moving this functionality there?
| @@ -134,15 +135,24 @@ UInt SpatialPooler::getNumInputs() const { return numInputs_; } | |||
| UInt SpatialPooler::getPotentialRadius() const { return potentialRadius_; } | |||
|
|
|||
| void SpatialPooler::setPotentialRadius(UInt potentialRadius) { | |||
| NTA_CHECK(potentialRadius < numInputs_); | |||
| NTA_CHECK(potentialRadius < numInputs_/2); | |||
There was a problem hiding this comment.
Instead of throwing error, this should truncate the radius to the number of inputs like the original constructor did: potentialRadius > numInputs_ ? numInputs_ : potentialRadius
Also, I think that the /2 is a mistake.
There was a problem hiding this comment.
Instead of throwing error, this should truncate the radius
I was thinking about this and I'd prefer not to (truncate). The user asks directly the API method (set*, constructor) to do something, and if the method cannot comply, I think we should let the user know ASAP, instead of silently modifying.
An example usecase is parameter optimization/swarming, where you'd waste cpu cycles evaluating range of values, while this might throw much earlier.
Of course, the negative effect is that many tests need to be updated to correct values.
Also, I think that the /2 is a mistake.
I think not. It's a radius, not diameter:
This parameter defines a square (or hyper square) area: a
column will have a max square potential pool with sides of
length (2 * potentialRadius + 1).
ok, on the other hand:
- If the topology is one dimensional, and the potentialRadius is 5, this
method will return an array containing 5 consecutive values centered on
the index of the column (wrapping around if necessary).
- If the topology is two dimensional (not implemented), and the
potentialRadius is 5, the method should return an array containing 25
'1's, where the exact indices are to be determined by the mapping from
1-D index to 2-D position.
both from SpatialPooler.hpp, so either doc is wrong.
|
I will load up my Linux and see if I can reproduce it.
It is in the area that I made a lot of changes.
…On Tue, Feb 26, 2019 at 4:10 AM breznak ***@***.***> wrote:
I'm getting a segfault in Debug on CppRegionImpl_ fanIn, @dkeeney
<https://github.com/dkeeney> you've been working with the code, could you
have a look if you have time, please?
[----------] 4 tests from CppRegionTest
[ RUN ] CppRegionTest.testCppLinkingFanIn
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff78e7198 in __gnu_debug::_Safe_sequence_base::_M_detach_all() ()
from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
(gdb) bt
#0 0x00007ffff78e7198 in __gnu_debug::_Safe_sequence_base::_M_detach_all()
() from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#1 <#1>
0x000055555591e951 in __gnu_debug::_Safe_sequence_base::_Safe_sequence_base
(this=0x555555d5ecb0, __in_chrg=) at
/usr/include/c++/7/debug/safe_base.h:221
#2 <#2>
__gnu_debug::_Safe_sequence<std::__debug::set<std::shared_ptrYAML::detail::node,
std::less<std::shared_ptrYAML::detail::node >,
std::allocator<std::shared_ptrYAML::detail::node > > >::_Safe_sequence
(this=0x555555d5ecb0, __in_chrg=) at
/usr/include/c++/7/debug/safe_sequence.h:108
#3 <#3>
__gnu_debug::_Safe_node_sequence<std::__debug::set<std::shared_ptrYAML::detail::node,
std::less<std::shared_ptrYAML::detail::node >,
std::allocator<std::shared_ptrYAML::detail::node > > >::_Safe_node_sequence
(this=0x555555d5ecb0, __in_chrg=) at
/usr/include/c++/7/debug/safe_sequence.h:131
#4 <#4>
__gnu_debug::_Safe_container<std::__debug::set<std::shared_ptrYAML::detail::node,
std::less<std::shared_ptrYAML::detail::node >,
std::allocator<std::shared_ptrYAML::detail::node >>,
std::allocator<std::shared_ptrYAML::detail::node >,
__gnu_debug::_Safe_node_sequence, true>::_Safe_container
(this=0x555555d5ecb0, __in_chrg=)
at /usr/include/c++/7/debug/safe_container.h:41
#5 <#5>
std::__debug::set<std::shared_ptrYAML::detail::node,
std::less<std::shared_ptrYAML::detail::node >,
std::allocator<std::shared_ptrYAML::detail::node > >::set (
this=0x555555d5ecb0, __in_chrg=) at /usr/include/c++/7/debug/set.h:120
#6 <#6>
YAML::detail::memory::memory (this=0x555555d5ecb0, __in_chrg=)
at
/mnt/store/devel/HTM/htm-community/nupic.cpp/build/ThirdParty/yaml-cpp/yaml-cpp-src/include/yaml-cpp/node/detail/memory.h:23
#7 <#7>
std::_Sp_counted_ptr<YAML::detail::memory*,
(__gnu_cxx::_Lock_policy)2>::_M_dispose (this=) at
/usr/include/c++/7/bits/shared_ptr_base.h:376
#8 <#8> 0x000055555563c981
in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release
(this=0x555555d83f00) at /usr/include/c++/7/bits/shared_ptr_base.h:154
#9 <#9> 0x000055555591e87e
in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count
(this=0x555555d83ee8, __in_chrg=) at
/usr/include/c++/7/bits/shared_ptr_base.h:684
#10 <#10>
std::__shared_ptr<YAML::detail::memory, (__gnu_cxx::_Lock_policy)2>::__shared_ptr
(this=0x555555d83ee0, __in_chrg=) at
/usr/include/c++/7/bits/shared_ptr_base.h:1123
#11 <#11>
std::shared_ptrYAML::detail::memory::shared_ptr (this=0x555555d83ee0,
__in_chrg=) at /usr/include/c++/7/bits/shared_ptr.h:93
#12 <#12>
YAML::detail::memory_holder::memory_holder (this=0x555555d83ee0,
__in_chrg=)
at
/mnt/store/devel/HTM/htm-community/nupic.cpp/build/ThirdParty/yaml-cpp/yaml-cpp-src/include/yaml-cpp/node/detail/memory.h:33
#13 <#13>
std::_Sp_counted_ptr<YAML::detail::memory_holder*,
(__gnu_cxx::_Lock_policy)2>::_M_dispose (this=) at
/usr/include/c++/7/bits/shared_ptr_base.h:376
#14 <#14>
0x000055555563c981 in
std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release
(this=0x555555d83f20) at /usr/include/c++/7/bits/shared_ptr_base.h:154
#15 <#15>
0x0000555555922437 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count
(this=0x7fffffffbf10, __in_chrg=) at
/usr/include/c++/7/bits/shared_ptr_base.h:684
#16 <#16>
0x0000555555925f70 in std::__shared_ptr<YAML::detail::memory_holder,
(__gnu_cxx::_Lock_policy)2>::__shared_ptr (this=0x7fffffffbf08,
__in_chrg=)
at /usr/include/c++/7/bits/shared_ptr_base.h:1123
#17 <#17>
0x0000555555925fb2 in
std::shared_ptrYAML::detail::memory_holder::~shared_ptr
(this=0x7fffffffbf08, __in_chrg=) at /usr/include/c++/7/bits/shared_ptr.h:93
#18 <#18>
0x0000555555981cd8 in YAML::NodeBuilder::~NodeBuilder (this=0x7fffffffbf00,
__in_chrg=)
at
/mnt/store/devel/HTM/htm-community/nupic.cpp/build/ThirdParty/yaml-cpp/yaml-cpp-src/src/nodebuilder.cpp:18
#19 <#19>
0x000055555592920e in YAML::Load (input=...) at
/mnt/store/devel/HTM/htm-community/nupic.cpp/build/ThirdParty/yaml-cpp/yaml-cpp-src/src/parse.cpp:24
#20 <#20>
0x0000555555928fce in YAML::Load (input="") at
/mnt/store/devel/HTM/htm-community/nupic.cpp/build/ThirdParty/yaml-cpp/yaml-cpp-src/src/parse.cpp:14
#21 <#21>
0x000055555591acec in nupic::YAMLUtils::toValueMap
(yamlstring=0x7fffffffcc60 "", parameters=..., nodeType="TestNode",
regionName="region1")
at
/home/mmm/devel/HTM/htm-community/nupic.cpp/src/nupic/engine/YAMLUtils.cpp:187
#22 <#22>
0x0000555555859999 in nupic::RegionImplFactory::createRegionImpl
***@***.***=0x555555d17360
nupic::RegionImplFactory::getInstance()::instance, nodeType="TestNode",
nodeParams="", ***@***.***=0x555555d84210) at
/home/mmm/devel/HTM/htm-community/nupic.cpp/src/nupic/engine/RegionImplFactory.cpp:120
#23 <#23>
0x000055555585551d in nupic::Region::Region (this=0x555555d84210, name=...,
nodeType="TestNode", nodeParams="", network=)
at
/home/mmm/devel/HTM/htm-community/nupic.cpp/src/nupic/engine/Region.cpp:72
#24 <#24>
0x0000555555839f13 in
__gnu_cxx::new_allocatornupic::Region::construct<nupic::Region,
std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&,
std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&,
std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&,
nupic::Network*> (
__p=0x555555d84210, this=) at /usr/include/c++/7/ext/new_allocator.h:136
#25 <#25>
std::allocator_traits<std::allocatornupic::Region
>::construct<nupic::Region, std::__cxx11::basic_string<char,
std::char_traits, std::allocator > const&, std::__cxx11::basic_string<char,
std::char_traits, std::allocator > const&, std::__cxx11::basic_string<char,
std::char_traits, std::allocator > const&, nupic::Network*> (
__p=, __a=...) at /usr/include/c++/7/bits/alloc_traits.h:475
#26 <#26>
std::_Sp_counted_ptr_inplace<nupic::Region, std::allocatornupic::Region,
(__gnu_cxx::_Lock_policy)2>::_Sp_counted_ptr_inplace<std::__cxx11::basic_string<char,
std::char_traits, std::allocator > const&, std::__cxx11::basic_string<char,
std::char_traits, std::allocator > const&, std::__cxx11::basic_string<char,
std::char_traits, std::allocato---Type to continue, or q to quit---q
Quit
(gdb)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#279 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFBa_6rceP2zcKY2H-vX_hwDprFr61lvks5vRSQ7gaJpZM4bSB8p>
.
|
Hmmm, this was a known bug that is deep within Yaml. I thought I had avoided it by pulling directly from the yaml repository rather from their latest release. Not sure what to do. Maybe I should go see if I can fix it for them? ... :) |
|
There is another alternative that might work. I can always build yaml-cpp as Release even if the main lib is being build as debug. We would not be debugging into yaml anyway. Let me see if that links. |
perfect! I was going to suggest that anyway, as yaml-cpp is the only one of externals that takes the Debug/Release parameter and is being rebuilt each time it changes. This is a win:win solution 🍰 |
|
Well, it almost works out-of-the-box by changing the BuildType in the bootstrap. But gtest needs to be debug. Let me fix this. |
|
Hmmmm, it still crashes. Even with the yaml library compiled as Release. |
👍 please publish your yaml-RELEASE as a separate PR, so we can merge it quickly
😿 we should report it to yaml-cpp upstream, but they have not been very responsive. Is it the same issue as we'd logged with them already?
can we use some alternative to yaml-cpp? What kind of data do we use it for? |
|
Yes, this will be in a separate PR. |
|
my new PR for investigating the yaml crash is PR #282 |
Fixes #5