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

Forward hooks and loggable/configurables. #9

Open
matham opened this issue May 11, 2023 · 0 comments
Open

Forward hooks and loggable/configurables. #9

matham opened this issue May 11, 2023 · 0 comments

Comments

@matham
Copy link
Contributor

matham commented May 11, 2023

Other than streaming intermediate states to disk, what else would you use forward hooks for?

Well, it saves having to pass state around!? And users can perhaps decide which states to save in the hook, rather than always getting and passing the state around so that you're always paying the cost for everything. By using a forward hook, you leave it all to the hook and you can use different hooks on the user side, unlike passing the loggables around, which you do at the framework level and users have no control over.

This is part of the convo about Loggable and Configurable. I think how you saw loggable is different than what I had initially intended it as. As I had originally intended, unlike Configurable where the property value is stored in the object, loggable doesn't store the exact properties to be logged as that was stored in a separate dict. That is, like configurable, _loggable_props_ was meant as a hard list of all the potential props that can be logged. However, to decide at runtime which props are actually logged, that's what I had a yaml file for, where each potential loggable property is marked whether to be logged or not. And, during runtime, the dicts created from the yaml can be dynamically updated to change some loggables from True to False etc.

Your interpretation instead seems to be that everything listed in loggable_props are to be logged. And if we need to change that, then we need to update loggable_props. Vs. updating a separate dict and keeping loggable_props unchanged.

This is where some of that tension of how you are using loggable_props and extensible_props is coming from I think.

Anyway, that's all to say, if you did follow my approach, then blindly saving all loggables after every forward at a framework level may be wasteful as users may not want to log everything listed in loggable. And using customizable forward hooks may be more flexible.

On the other hand, users can dynamically override so that loggable_state() returns different things dynamically, maybe based on a filter dict changing which props are logged for each forward call, so perhaps this is not an issue. This is just something for you to consider.

Originally posted by @matham in #1 (comment)

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

1 participant