search

opennars / Narjure

A Clojure implementation of the Non-Axiomatic Reasoning System proposed by Pei Wang.
GNU General Public License v2.0
44 stars 11 forks source link

Bag rmoving unrelated elements on add-element #37

Closed TonyLo1 closed 8 years ago

TonyLo1 commented 8 years ago

See this gist for details: https://gist.github.com/TonyLo1/3a461e5e4fd8a3e675298d3a22de59ad

Summary, on full bag adding elements with existing :id causes unrelated elements to be removed. For example adding :id 0 with new :priority causes :id 0 to be updated but :id 2 to be removed (see gist)

patham9 commented 8 years ago

There is an exists? function. If when this one returns true, update-element is called, and if false, add-element as usual, the issue does not arise. To not have this logic at caller side, I suggest adding this check in add-element itself.

patham9 commented 8 years ago

For completeness, the behavior can be explained by this:

At the stage of adding the new element of existing ID, the bag is full. Which means add-element triggers popping an element (altough the ID exists): It removes one element of the list of lowest priority elements (which are multiple because all have 0.1 priority), in this case it removes the id 4 element, and adds the new element of existing ID ( (add-element bag' element) ).

The real issue now is that we end up with a not sorted priority-index and one element too much removed, the latter happens because of the previous step, and the former because the following happens: priority-index' (conj priority-index (el id priority)) This does not update the order of the existing ID instead it just overwrites its priority.

So two possible solutions are:

  1. Always update the bag according to the ID of the added element, which is expensive. (not an option) RECOMMENDED: 2. Apply the exists check in adding as suggested before, which is cheap and will only apply the needed disaccociation+association as demanded for getting re-sorted when really needed. 3dd0cd4e4c58abaf1f83231bcb6b44a7dcafcc8d (elegant)
  2. Only use the exists check + diassociation+association part of update-element additionally in add-element as my commit f13c24b5667469fa64b6e7efee721748ff918e21 suggests. (spaghetti code)
patham9 commented 8 years ago

Related pull request: https://github.com/opennars/opennars2/pull/38 Will close the issue when this one has be merged.