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

Documentation: Error and lack of clarity in description of models #472

Open
Make42 opened this issue Feb 14, 2023 · 6 comments
Open

Documentation: Error and lack of clarity in description of models #472

Make42 opened this issue Feb 14, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@Make42
Copy link
Contributor

Make42 commented Feb 14, 2023

In https://clear.ml/docs/latest/docs/clearml_sdk/model_sdk/ it says

  • Model - represents an experiment's output model (training results). An OutputModel is always connected to a task.
  • InputModel - represents an existing ClearML model to be used in an experiment.
  • OutputModel - represents a ClearML model, regardless of any task connection.

Where is OutputModel mentioned in the first point? There is some error - not sure what is wrong though. Maybe the texts for Model and OutputModel are switched?

@Make42 Make42 added the bug Something isn't working label Feb 14, 2023
@ainoam
Copy link
Collaborator

ainoam commented Feb 14, 2023

Thanks for spotting @Make42! Indeed the texts have been switched.

I'm moving this issue to the docs repo. If you like, you can issue a PR to fix :)

@ainoam ainoam transferred this issue from allegroai/clearml Feb 14, 2023
@Make42
Copy link
Contributor Author

Make42 commented Feb 15, 2023

Also, I am really not clear on what the difference between the model classes are. As far as I understand, the workflow is like this:

  1. I define some model. Then I register it as an OutputModel. Then I train it. During training I save snapshots (not idea how, though) and then I save the final model when training is finished. This way the Model is a) connected to the task and b) available in the model store of ClearML.
  2. Later, in a different task, I can load an already trained model with InputModel. This InputModel is read-only (regarding the ClearML model store), but I can make a copy on which I work and - e.g. - further train. So, what I would think, I get the model with InputModel from the model store, make a copy, register the copy with my new task as an OutputModel and then go on as I did in the last paragraph 1.

No idea, how Model comes into play here. Let me compare Model:

class Model()
Represent an existing model in the system, search by model id. The Model will be read-only and can be used to pre initialize a network

with InputModel

class InputModel()
Load an existing model in the system, search by model ID. The Model will be read-only and can be used to pre initialize a network. We can connect the model to a task as input model, then when running remotely override it with the UI.
Load a model from the Model artifactory, based on model_id (uuid) or a model name/projects/tags combination.

Sounds just the same to me. What is the difference?

I would like to ask for this to be documented in more detail. :-)

@Make42 Make42 changed the title Documentation: Error in description of Models Documentation: Error and lack of clarity in description of models Feb 15, 2023
@ainoam
Copy link
Collaborator

ainoam commented Feb 15, 2023

Apologies for the ambiguity @Make42, the class hierarchy is indeed somewhat messy, and we have plans to simplify it in future versions.
In the mean time, let me try to clarify:

  • Model : This class is the basic interface to the ClearML model store Use it to programmatically access and manage the model store.
  • InputModel : Use this class when the model is used in the context of a ClearML experiment, as it's initial starting point.
  • OutputModel : Use this class for a model that is the result of your ClearML experiment.

We'll make a point to improve the docs in this regard :)

Make42 added a commit to Make42/clearml-docs that referenced this issue Feb 16, 2023
Fixes the swap in description, that is mentioned in allegroai#472
@Make42
Copy link
Contributor Author

Make42 commented Feb 16, 2023

@ainoam : I started fixing the issue with the pull request I just made. However, this only contributes to the swap mentioned at the beginning. Please add more clarity on the problem, as I have described it in my later posts.

ainoam pushed a commit that referenced this issue Feb 16, 2023
Fixes the swap in description, that is mentioned in #472
@ainoam
Copy link
Collaborator

ainoam commented Feb 20, 2023

Thanks @Make42. You'll note #479 indeed addresses the extra details discussed, so this issue is taken care of?

@Make42
Copy link
Contributor Author

Make42 commented Feb 21, 2023

@ainoam: Are all the changes what I can see at https://github.com/allegroai/clearml-docs/pull/479/files?
This is helpful, because I see now, what the general idea is and when I am supposed to use each class.

It still leaves open the question, what the capability each of those have - in other words - why the architects decided to create three classes instead of only two. I somewhat got it after re-reading the text in https://clear.ml/docs/latest/docs/clearml_sdk/model_sdk and compared the available methods and their signatures in https://clear.ml/docs/latest/docs/references/sdk/model_model/ and https://clear.ml/docs/latest/docs/references/sdk/model_inputmodel, but maybe this can be made more explicit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants