-
Notifications
You must be signed in to change notification settings - Fork 22
Description
Hi Harald,
First off, let me say this is a fantastic project! I currently reimplement WordDetectorNN for a hobby project of mine (Xournal++ HTR, see corresponding feature branch) and I love this project of yours and your work in general!
While understanding and reimplementing the code, I noticed a small area for improvement in the validate function within src/train.py. The scalar validation metrics (val_loss, val_f1, etc.) are currently logged to TensorBoard on every iteration of the image loop. This causes the same metric values to be written multiple times for a single validation step. I suspect this is an indentation mistake?
Current Code in src/train.py:
def validate(net, loader, writer):
# ... (evaluation logic) ...
res = evaluate(net, loader, max_aabbs=1000)
for i, (img, aabbs) in enumerate(zip(res.batch_imgs, res.batch_aabbs)):
vis = visualize(img, aabbs)
writer.add_image(f'img{i}', vis.transpose((2, 0, 1)), global_step)
# These lines are logged for every image in the batch
writer.add_scalar('val_loss', res.loss, global_step)
writer.add_scalar('val_recall', res.metrics.recall(), global_step)
# ... and so onSuggested Fix:
Moving the scalar logging outside of the loop would make the TensorBoard logs cleaner.
def validate(net, loader, writer):
# ... (evaluation logic) ...
res = evaluate(net, loader, max_aabbs=1000)
# Log image visualizations
for i, (img, aabbs) in enumerate(zip(res.batch_imgs, res.batch_aabbs)):
vis = visualize(img, aabbs)
writer.add_image(f'img{i}', vis.transpose((2, 0, 1)), global_step)
# Log scalar metrics once per validation run
writer.add_scalar('val_loss', res.loss, global_step)
writer.add_scalar('val_recall', res.metrics.recall(), global_step)
# ... and so onIt's a minor point, but I hope this feedback is helpful. If you agree with the change, I would be happy to create a small pull request to address it.
Thanks again for all your work & cheers! :-)