mittagessen / kraken

OCR engine for all the languages
http://kraken.re
Apache License 2.0
750 stars 131 forks source link

Add tensorboard logging to train and segtrain #428

Closed colibrisson closed 1 year ago

colibrisson commented 1 year ago

This pull request adds Tensorboard logging as proposed in https://github.com/mittagessen/kraken/issues/422.

Tensorboard logging is triggered by adding a --debug flag to the ketos train and ketos segtrain commands.

colibrisson commented 1 year ago

I haven't used ketos pretrain yet. I will try to implement Tensorboard logging for it in a near future. Maybe next week.

As far as I understand, in ketos train, val_metric is the same as val_accuracy. In master, they are currently logged as follows:

        self.log_dict({'val_accuracy': accuracy,
                       'val_metric': accuracy}, prog_bar=True)

Since it's used for early stopping, I kept val_metric. But I thought it wasn't necessary to log it to Tensorboard.

PonteIneptique commented 1 year ago

My previous mail seems to have not been registered by GitHub. May I join the conversation or propose to use - - log tensor board rather than - - debug which kind of makes - - raise-on-error weird?

On Sat, 4 Feb 2023, 10:54 pm Colin Brisson, @.***> wrote:

I haven't used ketos pretrain yet. I will try to implement Tensorboard logging for it in a near future. Maybe next week.

As far as I understand, in ketos train, val_metric is the same as val_accuracy. In master, they are currently logged as follows:

    self.log_dict({'val_accuracy': accuracy,
                   'val_metric': accuracy}, prog_bar=True)

Since it's used for early stopping, I kept val_metric. But I thought it wasn't necessary to log it to Tensorboard.

— Reply to this email directly, view it on GitHub https://github.com/mittagessen/kraken/pull/428#issuecomment-1416858316, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOXEZWHRBHZYNSIB32E2RLWV3FZ7ANCNFSM6AAAAAAURMMX24 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

colibrisson commented 1 year ago

You are making a good point but I can't find --raise-on-error. Is it a ketos option?

mittagessen commented 1 year ago

On 23/02/05 07:20AM, Colin Brisson wrote:

You are making a good point but I can't find --raise-on-error. Is it a ketos option?

It is an option on kraken, not ketos.

PonteIneptique commented 1 year ago

My bad then. I still feel like - - debug has a more specific semantic than - - log tensorboard but my point is weaker :/

On Sun, 5 Feb 2023, 4:36 pm mittagessen, @.***> wrote:

On 23/02/05 07:20AM, Colin Brisson wrote:

You are making a good point but I can't find --raise-on-error. Is it a ketos option?

It is an option on kraken, not ketos.

— Reply to this email directly, view it on GitHub https://github.com/mittagessen/kraken/pull/428#issuecomment-1418053587, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOXEZR7RJFGOEGITF4KO7DWV7CIVANCNFSM6AAAAAAURMMX24 . You are receiving this because you commented.Message ID: @.***>

colibrisson commented 1 year ago

You are absolutely right, --debug meaning is broader. When I proposed this flag, I thought Tensorboard could help to find out what happens when training doesn't go as expected, thus --debug training. Maybe a simple --tensorboard flag would be better.

colibrisson commented 1 year ago

Or maybe the --debug flag could print debug-level logs on the console and trigger Tensorboard logging at the same time.

mittagessen commented 1 year ago

On 23/02/05 07:42AM, Thibault Clérice wrote:

My bad then. I still feel like - - debug has a more specific semantic than - - log tensorboard but my point is weaker :/

Yeah, I'd just call it --log-dir or --tensorboard-log-dir or something like that. --debug feels too much like running at a higher verbosity level or something.

PonteIneptique commented 1 year ago

--log tensorboard would also allow for something like --log wandb down the line ?

colibrisson commented 1 year ago

I propose using two options:

  1. --logger indicates the logger used. Default is Tensorboard but support for W&B WanDB or Neptune could be added in the future.
  2. --log-dir indicates where the logs are saved.
PonteIneptique commented 1 year ago

I like that

On Sun, 5 Feb 2023, 7:09 pm Colin Brisson, @.***> wrote:

I propose using two options:

  1. --logger indicates the logger used. Default is Tensorboard but support for W&B WanDB or Neptune could be added in the future.
  2. --log-dir indicates where the logs are saved.

— Reply to this email directly, view it on GitHub https://github.com/mittagessen/kraken/pull/428#issuecomment-1418204234, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOXEZV7JBFCC77ZGGTIADTWV7UEHANCNFSM6AAAAAAURMMX24 . You are receiving this because you commented.Message ID: @.***>

mittagessen commented 1 year ago

I think the lightning cli options for logging work the same way. Let's do that then.

colibrisson commented 1 year ago

As we discussed, I have updated the cli options for Tensorboard logging.

mittagessen commented 1 year ago

Thanks.