Skip to content
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

Remove LearnLogger? #61

Open
ericphanson opened this issue May 2, 2022 · 3 comments
Open

Remove LearnLogger? #61

ericphanson opened this issue May 2, 2022 · 3 comments

Comments

@ericphanson
Copy link
Member

After #60 we've defined an interface that folks can plug into for custom logging.

We could drop the Tensorboard dep by removing the LearnLogger. We could make a no-op logger that doesn't actually log anything, or maybe a Dict logger that logs things to a logged dict as the current one does, which could be good for testing. Or we could keep the current LearnLogger.

@hannahilea
Copy link
Contributor

maybe a Dict logger that logs things to a logged dict as the current one does, which could be good for testing

I vote this one!

And then either drop [tb] LearnLogger entirely, or perhaps add it to the docs or tests as a (tested?) example? It feels bad to have to keep supporting it, though, since as far as we know no one uses it. Maybe dropping it is fine, esp when our current internal logger is public (soon?) and we can point at it as an example of how to implement the interface.

@ericphanson
Copy link
Member Author

One other option could be to have LearnLogger wrap other loggers. So it would be like

struct LearnLogger{L}
    logger::L
    logged::Dict{String,Vector{Any}}
end

and it could log everything to logged and then forward the call onto logger... So like

function log_value!(logger::LearnLogger, field, value)
    values = get!(() -> Any[], logger.logged, field)
    push!(values, value)
    return log_value!(logger.logger, field, value)
end

Then a LearnLogger is just something that wraps a logger to store the info to a Dict as well. Not really sure if we would actually use that or not though. I think the Dict-based thing is kinda problematic for huge training jobs because you need to store everything in memory.

@hannahilea
Copy link
Contributor

I think the Dict-based thing is kinda problematic for huge training jobs because you need to store everything in memory.

Yeah---and I think if someone does want to log to a local dict, they can easily make a logger that does that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants