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

Don't start Usage with Keras exampe #87

Closed
daavoo opened this issue Jun 10, 2021 · 5 comments
Closed

Don't start Usage with Keras exampe #87

daavoo opened this issue Jun 10, 2021 · 5 comments
Labels
A: docs Area: user documentation

Comments

@daavoo
Copy link
Contributor

daavoo commented Jun 10, 2021

The current dvclive Usage Guide directly jumps into the keras integration omitting the possibility of it's most basic usage (as a python library). Likewise the Dvclive with DVC is also "tied" to this keras example.

I think that it might be better to start the Usage Guide with a standalone approach (something similar to this existing section of the README) and a separate page explaining the usage along with DVC and link to a new page called Integrations.

The current content of the Usage Guide could be moved to a new Integrations/keras section. And we might create new pages for other undocumented integrations like xgboost or mmcv. Further integrations could be simply appended to Integrations.

Something like:

dvclive/
├── Integrations
│   ├── keras
│   ├── mmcv
│   └── xgboost
└── Usage
    ├── StandAlone
    └── WithDVC
@daavoo daavoo changed the title dvclive: Update Usage Guide dvclive: Update "Usage Guide" Jun 10, 2021
@jorgeorpinel
Copy link

jorgeorpinel commented Jun 11, 2021

I think that it might be better to start the Usage Guide with a standalone approach

This makes sense to me. I'd start with this.

The current content of the Usage Guide could be moved to a new Integrations/keras section

I don't think it's really about integrating with Keras though, or that we want to add specific integration pages (for now). That would be a separate question at least.

So hopefully the Keras example can remain as an "advanced" section in the usage guide for now? (reused in the
"Using with DVC" page)

@jorgeorpinel

This comment has been minimized.

@jorgeorpinel jorgeorpinel transferred this issue from iterative/dvc.org Jun 12, 2021
@jorgeorpinel jorgeorpinel added the A: docs Area: user documentation label Jun 12, 2021
@jorgeorpinel jorgeorpinel changed the title dvclive: Update "Usage Guide" Don't start Usage with Keras exampe Jun 12, 2021
@daavoo
Copy link
Contributor Author

daavoo commented Jun 17, 2021

I think that it might be better to start the Usage Guide with a standalone approach

This makes sense to me. I'd start with this.

I will start with that !

The current content of the Usage Guide could be moved to a new Integrations/keras section

I don't think it's really about integrating with Keras though, or that we want to add specific integration pages (for now). That would be a separate question at least.

I think that even if it the integrations feel like a separate subject, the documentation should revolve around it.

Reviewing other dvclive alternatives, it seems that they quite focused on showcasing how easy the integrations with other frameworks are. For example, integrations is the first thing mentioned in the "front page" of wandb (https://wandb.ai/site) . It is also a common pattern across different "vendors":

I think that this makes a lot of sense given that most of the potential users of a "ml logger" will be probably already adepts of one of more "ml training" framework and the "low level" API of a "ml logger" doesn't really matters to them.

So hopefully the Keras example can remain as an "advanced" section in the usage guide for now? (reused in the
"Using with DVC" page)

I usually see "advanced" sections as references for when the simplest solution doesn't cover my needs. However I feel like the current state of the keras example contains an unnecessary complexity that might mislead a potential user.

We are telling the user to write a custom keras callback when there is one already implemented and maintained in the repository. In contrast, the DVC 2.0 release blog post is more concise about this and tells the user that a single import is needed.

It would be useful to have an "advanced" section on how to write a custom integration, though.

@jorgeorpinel
Copy link

jorgeorpinel commented Jun 19, 2021

even if it the integrations feel like a separate subject, the documentation should revolve around it.

Hmmm maybe I'm confused about the term "integration" here. The only other software that DVCLive integrates with is DVC. Having usage examples with major frameworks does seem important too but again, let's make a separate issue for that? It shouldn't prevent us from keeping Keras in this main usage page (advanced section) IMO.

current state of the keras example contains an unnecessary complexity

Right so I think we've already agreed to simplify the usage example at first. But are you saying we don't need an advanced section at all then? Just remove all Keras specific code and leave that for some other section about "integrations"? For now I wouldn't do that but it can be addressed by the other issue I'm recommending to be open.

@daavoo
Copy link
Contributor Author

daavoo commented Jul 12, 2021

Right so I think we've already agreed to simplify the usage example at first. But are you saying we don't need an advanced section at all then?

And advanced section would still make sense, I guess, but I'm not sure what should it cover.

I see two types of use cases for dvclive; either you manually integrate the dvclive Python API in your custom training code ("Basic Usage") or you use dvclive along with one existing ML Framework ("Integrations").

Maybe the advanced section could dive into how to build new "Integrations"? Or "diving" into how some of the existing integrations are implemented (i.e. common Callback patterns used in ML Frameworks)?

Just remove all Keras specific code and leave that for some other section about "integrations"?

I think that we should go with that approach. An example of the unnecessary complexity that might mislead a potential user, already happened in one external repository where the user wrote it's own Keras callback instead of using the included in dvclive: sicara/dvc-streamlit-example#1

For now I wouldn't do that but it can be addressed by the other issue I'm recommending to be open.

I think I will close this issue and try to create the task list you suggested in #86

@daavoo daavoo closed this as completed Jul 12, 2021
@daavoo daavoo mentioned this issue Jul 12, 2021
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: docs Area: user documentation
Projects
None yet
Development

No branches or pull requests

2 participants