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

Model factory #351

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open

Model factory #351

wants to merge 31 commits into from

Conversation

AleShi94
Copy link
Collaborator

Possible solution to issue #286

  1. model_factory and model_factory_from_env can take external neural network.
  2. model_factory and model_factory_from_env can load a nn from file
  3. model_factory and model_factory_from_env can load a checkpoint for nn
  4. model_factory_from_env runs the check that the external nn satisfies requirements of the environment and model_configs

All new features are tested in test_torch_training.py file

SHILOVA Alena and others added 25 commits January 21, 2022 17:01
…and gives shorter example when it is function
…checks that externally defined nn is suitable for environment
@AleShi94 AleShi94 added ready for review Marathon To do during Marathon labels Jul 24, 2023
Copy link
Collaborator

@KohlerHECTOR KohlerHECTOR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I understand the if/else structure of the model factory. What happens if both a filename and nn.module instance are passed to the model factory function ?

@AleShi94
Copy link
Collaborator Author

I am not sure I understand the if/else structure of the model factory. What happens if both a filename and nn.module instance are passed to the model factory function ?

well, technically you can do it so that you take architecture from nn.Module and from the file you can take model weights. In that case it will load weights from the file and assign them to the architecture

But to be honest I am not 100% sure if it is the best way to code stuff, but the idea is that it should be flexible enough:
either you use available tools to build a nn (like it was done before), or you can propose your own architecture through net argument or you can upload your model or checkpoint of the model through a file, which should allow transfer learning

@KohlerHECTOR
Copy link
Collaborator

KohlerHECTOR commented Jul 24, 2023

Your idea is very clear and cool. What about only taking a nn module instance as input ? And providing the user with a simple pol net val net and a model factory ? So it is the user's task to build its networks the way he/she likes?
Otherwise: lgtm :)

@JulienT01
Copy link
Collaborator

JulienT01 commented Sep 20, 2023

I just added two one little comment.

And you need to run your tests (test_torch_training.py) inside functions that start with 'test_' (previous error, which needs to be corrected).

Otherwise it looks good to me. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Marathon To do during Marathon ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants