Feature/100 separate log files #143
Conversation
|
At the first glance this looks much more complicated than it should be. Please motivate why it was neccesarry |
art/art_logger.py
Outdated
| from loguru import logger | ||
| from pathlib import Path | ||
| from art.paths import LOG_PATH | ||
| logger.remove() |
There was a problem hiding this comment.
This just makes sure that default loguru handler is removed?
art/art_logger.py
Outdated
| from pathlib import Path | ||
| from art.paths import LOG_PATH | ||
| logger.remove() | ||
| logger.add(sys.stdout, format="{message}", level="DEBUG", filter= lambda record: ('halt_exception' not in record['extra']) or (not record['extra']['halt_exception'])) |
There was a problem hiding this comment.
This filter looks nice (It could even solve our lighnitng issue) but isn't it a big overhead to check in for everything that goes into it? Why this is necessary?
art/art_logger.py
Outdated
|
|
||
| art_logger = logger | ||
|
|
||
| if __name__ == "__main__": |
There was a problem hiding this comment.
If this is just for debugging purposes remove it
art/art_logger.py
Outdated
| yield | ||
| return | ||
| already_contextualized_logers.add(str(folder_path)) | ||
| global logger_level |
There was a problem hiding this comment.
shouldn't this be more number_of_loggers?
art/art_logger.py
Outdated
| try: | ||
| with logger.contextualize(file=str(folder_path)): | ||
| try: | ||
| yield |
There was a problem hiding this comment.
how one can obtain exception here?
art/paths.py
Outdated
|
|
||
| CHECKPOINTS_PATH = Path("checkpoints") | ||
| LOG_PATH = Path("logs") | ||
| EXPERIMENT_LOG_PATH = CHECKPOINTS_PATH/"experiment"/"logs" |
There was a problem hiding this comment.
The structure is okay but root folder should be user defined. What if I want to run 2 or more experiments in one folder. All will be in the same place?
I mean we have this exp1 defined in our first tutorial
There was a problem hiding this comment.
than it will be run in another folder so it will by in exp2
art/paths.py
Outdated
|
|
||
|
|
||
| def get_checkpoint_step_dir_path(step_id: str, step_name: str) -> Path: | ||
| """ |
There was a problem hiding this comment.
I like this is taken out from jsonsavers
| @@ -54,11 +56,12 @@ def __call__( | |||
| datamodule (L.LightningDataModule): Data module to be used. | |||
There was a problem hiding this comment.
I just wonder why can't we create custom logger here (we will have it's id), after everything is runned we can just remove it by id. We could generate random log name from tempfile library and link it to the results so log path will be available from json. With this we also have logs for every run stored
|
Moreover it is very important for me to create new log file for every run of experiment |
art/paths.py
Outdated
|
|
||
| CHECKPOINTS_PATH = Path("checkpoints") | ||
| LOG_PATH = Path("logs") | ||
| EXPERIMENT_LOG_PATH = CHECKPOINTS_PATH/"experiment"/"logs"/"logs.log" |
test fix
redundant log path delete
No description provided.