-
Notifications
You must be signed in to change notification settings - Fork 119
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
Better output when convergence check succeeds #150
Comments
I would like to take up this issue. |
Hi @vikram2000b, you're more than welcome to. When you have a PR ready, someone will review it when they're able to. 👍 |
Can I take up this issue as there is no PR yet? |
@dB2510 I think that is reasonable, @vikram2000b are you fine with that? |
@zoq Any idea how can I reproduce the issue mentioned here mlpack/mlpack#2073 ? |
@dB2510 there is some code in that issue that you can directly compile and use in a program to reproduce the issue. You will probably have to find some datasets to use to reproduce the issue, but that shouldn't be hard---you can probably use some of the datasets in |
Hey, @rcurtin wanted to know if this has been solved if not then can I move forward with it? |
@gaurav-singh1998 I have yet to see any PR related to the issue, so yes, if you can solve the issue, please do. 👍 |
Hi, @rcurtin @zoq while I was through the callback codebase trying to understand it for solving this issue I came across the piece of code given below which iterates over the callbacks and invoke the
I am having a hard time understanding how does this work. Please help understand the underlying concepts behind this. Thanks. |
@rcurtin @zoq one more thing, sorry if I irritate you but your help would be very helpful for moving forward with this issue. According to the discussion which happened here, the optimization terminates partway because of the |
Hey @gaurav-singh1998, the bit of code in I would suggest finding a way to make it so that |
Thanks, @rcurtin for the clarification I will try to solve this issue in the next few days. |
Hi, @rcurtin while I was going through the |
It does seem to me like However, I'm not familiar enough with the code to say for sure. Before making any changes, I would suggest first confirming that this is an issue by creating an example where |
Hi, @rcurtin @zoq @shrit according to the discussion that happened here I recreated the issue, the code regarding it can be found here. The dimensions of the dataset I used was
As it can be seen that not a single epoch was completed by the training process and according to me this is happening only because of the entire dataset not is being read by the optimizer. I am saying this because,
So, I don't think that the optimizer is reaching convergence to create this issue it is only happening because the optimizer is not parsing the entire dataset. I have tried to solve this issue here (Kindly review it). This being mentioned I think there are some issues in progress_bar.hpp which needs attention. One of them is the number of epochs not being correctly set as I mentioned above and the |
@gaurav-singh1998 I do not think that the optimizer has converged in your case, but this is only because you have set I am not familiar with |
Hi, @shrit thanks for commenting as suggested by you I changed the
As it can be seen that there is no message that states the optimizer has converged which should've got printed according to this but when I changed
This is weird because I think that the error lies in
Yes I used it and the warning message gets printed as expected. Kindly let me know if I am wrong somewhere. Thanks. |
@gaurav-singh1998 I do not remember if |
Hi, @shrit sorry to respond late but when I made the changes in this line to make the
Although the first line, |
Issue description
When using callbacks like
ens::ProgessBar
or similar, and the optimization terminates partway through an epoch due to a convergence check, the output can be confusing:but then output stops.
See the discussion at mlpack/mlpack#2073 for many more details and how to reproduce.
Expected behavior
We should see if we can modify the
ens::ProgressBar
callback (and perhaps others) to give better output in these situations. Perhaps something like this would be an improvement:Actual behavior
Instead the output just terminates:
This would be a good issue if you are not familiar with how ensmallen's callbacks work and would like to be. I don't have a direct route to a solution here, so my suggestion would be to investigate the current callback code, reproduce the issue, then think about the cleanest way to print convergence information in the
ens::ProgressBar
callback. Once that's done, see if similar changes might be useful for some of the other callbacks that print information (see http://ensmallen.org/docs.html#callback-documentation for more information on the callbacks that are available).The text was updated successfully, but these errors were encountered: