search

opencog / link-grammar

The CMU Link Grammar natural language parser
GNU Lesser General Public License v2.1
389 stars 119 forks source link

tracon-set.c grow_table(): Use MAX_TRACON_SET_TABLE_SIZE #1517

Closed ampli closed 7 months ago

ampli commented 7 months ago

This fixes a bug in tracon-set.c, existing there since I created it. The use of MAX_STRING_SET_TABLE_SIZE in grow_table() causes an inconsistent setting of the maximum loading factor, as tracon_set_create() and tracon_set_reset() use MAX_TRACON_SET_TABLE_SIZE and these definitions are now not the same.

After this fix, the maximum load factor of the tracon-set is always 3/8, as specified in tracon-set.h (in string-set.h and string-id.h it got set to 3/4 in commit 397a3d46). The run time doesn't noticeably improve, which may hint at a possible increase of MAX_TRACON_SET_TABLE_SIZE.

The unintentional use of MAX_STRING_SET_TABLE_SIZE went unnoticed because string-set.h is pulled into tracon-set.c through the inclusion of connectors.h. It would be better if internal module definitions were kept private and did not appear in internal API include files. So I propose to move the private string-set.c definitions from string-set.h to string-set.c, and the like for the other modules.

(This line is for a backreference in issue #1487, in which I mentioned this PR.)

linas commented 7 months ago

So I propose to move the private string-set.c definitions from string-set.h to string-set.c

Yes, this seems like a good idea. I was surprised to see the similar names in the header file, and kept thinking "this is dangerous and will lead to bugs" but I didn't actually try to fix it.