amharrison / jactr

-old - jACT-R Bundles
http://jact-r.org/
7 stars 5 forks source link

Please confirm test failure for DeclarativeModuleTest #25

Closed monochromata closed 8 years ago

monochromata commented 8 years ago

I'm currently working on the tests in org.jactr.core. The test org.jactr.core.module.declarative.DeclarativeModuleTest.testExactMatch fails in my repo. This might be due to my refactoring, but the code indicates it might fail even without the refactoring.

The test case compares chunk slot values via "a < 4" and expects 3 results, but there is actually a fourth one - a chunk with a=4. In org.jactr.core.module.declarative.search.map.NumericTypeValueMap.lessThan(Object) there is the following code:

double val = asKeyType(value) + EPSILON;
return getValueMap().lessThan(val);

EPSILON is 0.0001, hence "a < 4.0001" is true even for the fourth chunk with a=4. Maybe this code way added with floats in a special use-case in mind?

Could you check, whether the test case fails in your repo, too? If it does: let me know how to modify the implementation to cover the existing test case as well as the use case which caused EPSILON.

amharrison commented 8 years ago

I'll review it as soon as I can tomorrow. Most likely it is a straight up bone-headed bug that my poor testing discipline has let slip through.

On Sun, Apr 24, 2016 at 2:41 PM, monochromata notifications@github.com wrote:

Assigned #25 https://github.com/amharrison/jactr/issues/25 to @amharrison https://github.com/amharrison.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/amharrison/jactr/issues/25#event-639151345


Anthony M. Harrison

amharrison commented 8 years ago

Yup. The EPSILON was an old hold over from earlier float precision issues. It has been removed. (bug 1) But the partial matching tests were also incorrectly specified. Without their IChunkFilter, they would allow complete mismatches through (PartialMatchingChunkFilter requires that at least one feature of the search match).

I've made the necessary corrections in my repo to jactr.test and jactr.core