search

amharrison / jactr

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

Use of age offset can result in precision errors in the timed event queue #26

Open amharrison opened 8 years ago

amharrison commented 8 years ago

This is a critical bug. But only affects those models that are longitudinal, or assign a specific age to the model.

A model's age at the start of a run is used as the local clock offset. When a 100s old model waits for time 101, it's actually waiting for (localClock - offset), or 1s on the global clock.

From the model side, all waitFor and getTime() subtract or add the offset, respectively. The BasicClock mechanism automatically clamps values to a pre-specified precision, but we are still getting precision errors, such that errors like this are seen:

ERROR 10:14:06,275(model) [org.jactr.core.runtime.DefaultModelRunner.waitForClock] WARNING: Time discrepancy detected. Clock regression : 204.9541000000(returned) < 204.9541000000(desired). Should be >=

We can introduce an epsilon in the equality test, but that will likely cause more problems elsewhere.

Damn you floating point arithmetic.

monochromata commented 8 years ago

Hm, I didn't yet understand that error message. But I also didn't look at that code too intensely.

Do you think it could make sense (i.e. solve the issue and not consume infinite amounts of time) to use BigDecimal instead of double to represent and calculate time in CR and jACT-R?

amharrison commented 8 years ago

Yeah. Not the clearest error message. That error message is very high up and is just a sanity check on that time is increasing. It arises just at the clock interface level, BasicClock specifically. Fortunately, this means the fix is completely local.

First I need to confirm that this assessment is correct. The challenge is that it arises so rarely. Then I can research the 'proper' solution using intervals (damned, epsilons again).

I want to avoid BigDecimal if possible. The overhead may be small, but time calculations are everywhere. That will add up quickly.

monochromata commented 8 years ago

In my repo org.jactr.core.runtime.GeneralExecutionTest.testMultiple() generates this error relatively often (i.e. in 80% of all test runs, I would guess, there is effectively an infinite loop with this error message). I'm not sure whether in this test the error message might be related to an interference between the two models running concurrently. Of course in my repo the problems could all be due to my refactorings :-).

amharrison commented 8 years ago

I'm going to feel like such an idiot if the replication has been sitting under my nose. :)

On Wed, Apr 27, 2016 at 3:28 PM, monochromata notifications@github.com wrote:

In my repo org.jactr.core.runtime.GeneralExecutionTest.testMultiple() generates this error relatively often (i.e. in 80% of all test runs, I would guess, there is effectively an infinite loop with this error message). I'm not sure whether in this test the error message might be related to an interference between the two models running concurrently. Of course in my repo the problems could all be due to my refactorings :-).

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/amharrison/jactr/issues/26#issuecomment-215202157

Anthony M. Harrison