Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Request] Force Warning for Non-convergence #24

Closed
clukewatson opened this issue Jun 23, 2020 · 4 comments
Closed

[Request] Force Warning for Non-convergence #24

clukewatson opened this issue Jun 23, 2020 · 4 comments

Comments

@clukewatson
Copy link

Howdy,

FEOLS appears not to default issue a warning about non-convergence. I only realized this was an issue when I used verbose and adjusted the iteration limit. Even this does not directly tell me it did not converge, but I can see in the output that it hit its iteration limit.

I believe making non-convergence more salient would help most users.

Thank you for your efforts,
Best,
Luke

@lrberge
Copy link
Owner

lrberge commented Jun 23, 2020

Hi Luke, that's a very fair request!

Out of curiosity: do you happen to have variables with varying slopes in your estimation?

It's because the only situations I experienced in which the algorithm hit the iteration limit was when there were several variables with varying slopes. It's a known issue with a known fix (I still have to implement it though). Currently there's a warning in case of convergence problems but only in those situations.

As suggested, I'll add a warning in case of non-convergence and will also increase the number of iterations by default. (Actually I didn't implement it because I thought--more precisely hoped [no doubt too strongly]--that 2000 iterations would always be enough.)

@clukewatson
Copy link
Author

Yes, I did (or at least this was the case that I noticed the issue).

Awesome. Do you want me to close the issue, or I can leave that up to you?

@lrberge
Copy link
Owner

lrberge commented Jun 23, 2020

Thanks, I'll close it once I've made the changes in the code. In the meantime it serves me as a reminder.

@lrberge
Copy link
Owner

lrberge commented Jul 7, 2020

I've added a warning and the message will also pop in the print to really signal the problem.
Since there's not point to be fast if the results are wrong :-D, I've also increased the default number of iterations to 10,000.
Thanks again!

@lrberge lrberge closed this as completed Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants