spoonsso / dannce

MIT License
214 stars 30 forks source link

Logging #86

Closed diegoaldarondo closed 2 years ago

diegoaldarondo commented 2 years ago

Can we roll in the logging or absl.logging modules to better log our debugging print statements? Would help with sharing our log files, and reduce the printed output when not debugging.

data-hound commented 2 years ago

Yes, it would be good to have logging statements instead of print statements. For the choice of using absl for logging, it makes sense to add this dependency only if we want to use its unit testing framework as well. Are we considering absl for unit tests as well?

diegoaldarondo commented 2 years ago

absl is currently required for cluster/multi_gpu_test.py and tests/cli_test.py on the as_landing and automated_multi_instance_refactor branches. I don't think either testing suite requires anything from absl that isn't supported by unittest, so we could just change all references to absl to use unittest equivalents and skip the added dependency until we need additional absl functionality.

spoonsso commented 2 years ago

@data-hound any update on this?

data-hound commented 2 years ago

The code changes with logging statements are in as_logging. But, they have not been tested, yet. Once I test and verify things are working as expected, it will be good to merge.

data-hound commented 2 years ago

I have finished testing the as_logging branch using a dannce-predict run