search

signaflo / java-timeseries

Time series analysis in Java
MIT License
195 stars 49 forks source link

Handle Thread.interrupt in case of falling into infinite loop #3

Closed Ajk4 closed 6 years ago

Ajk4 commented 6 years ago

When grid searching over many parameters sometimes loops in BFGS class will never terminate.

Perhaps loops at BFGS#124 should scale the step up if change sign keeps the same?

For now I add Thread.interruption handler so clients code can gracefully stop code from running when it runs into infinite loop.

Ajk4 commented 6 years ago

Hi @signaflo

What do you think about it?

Probably ideally we would like to write the code in the way that it never goes into infinite loop. Regardless though I feel like having this safety valve of checking for thread interruptions in while loops is good idea anyways.

Ajk4 commented 6 years ago

I guess we could also add loop counter there and have maxIterations there as well.

signaflo commented 6 years ago

@Ajk4 I'm not a fan of checked exceptions except in cases where they absolutely can't be avoided. I'd prefer to deal with the source of the problem, which is a failure inside the optimization code. I like the idea of having maxIterations, which really should have been there in the first place and I apologize for missing that.

I've investigated a bit and the infinite loop occurs because of the introduction of Double.NaN values at this point in the algorithm. There are three options I can think of for dealing with this, keeping in mind that the whole point of this method is to estimate parameter values.

  1. Jump out of the entire loop and use the initial estimates as the final estimates.

  2. Jump out of the entire loop and use the most recent finite estimates as the final estimates (this is probably far preferable to option 1 since we have usually at least made significant progress at this point).

  3. Figure out why the algorithm is producing these NaN values and then figure out how to deal with it. This would probably be by far the most challenging and time consuming option.

Instead of merging I'd like to create an issue for this and then we can create a branch off of the develop branch for making the changes. I can get a quick fix in immediately and then decide what to do for a long-term fix.

Ajk4 commented 6 years ago

@signaflo

I can get a quick fix in immediately and then decide what to do for a long-term fix.

If you could do that, I would appreciative it a lot! Thanks!

Instead of merging I'd like to create an issue for this and then we can create a branch off of the develop branch for making the changes.

Sure. Thread.interrupt handling is a quick hack that works for me. I am okay with other approaches.

I added an issue already -> https://github.com/signaflo/java-timeseries/issues/4

If I will be able to provide some reproduction case I will post it ASAP.

signaflo commented 6 years ago

Closing pull request and moving discussion to issue #4. Issue somewhat resolved by release 0.4.