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

Load models in finetune mode based on command line parameters. #7329

Closed
3 tasks
wochinge opened this issue Nov 20, 2020 · 9 comments · Fixed by #7456
Closed
3 tasks

Load models in finetune mode based on command line parameters. #7329

wochinge opened this issue Nov 20, 2020 · 9 comments · Fixed by #7456
Assignees
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework type:enhancement ✨ Additions of new features or changes to existing ones, should be doable in a single PR

Comments

@wochinge
Copy link
Contributor

Load models for incremental training.

Depends on #7328.

  • load NLU pipeline in fine-tune mode as shown here
  • load Core policies in fine-tune mode as show here
  • pass in new number of epochs
@wochinge wochinge added type:enhancement ✨ Additions of new features or changes to existing ones, should be doable in a single PR area:rasa-oss 🎡 Anything related to the open source Rasa framework priority:high labels Nov 20, 2020
@dakshvar22
Copy link
Contributor

@wochinge @joejuzl Do you folks know how the load method for each ML component will be called during finetuning? For example, would there be an extra parameter passed to load function of the components to load them in "finetune" mode and also pass other configurations parameters like number of epochs? Here's how I had done it in the working version branch but I am assuming it wouldn't be exactly the same. If we can plan this ahead, it would unblock me on some of the implementation due for my PR. 🙏

@joejuzl
Copy link
Contributor

joejuzl commented Dec 1, 2020

@dakshvar22 I haven' started looking at this area of the code yet (working on #7330 first) so it's hard for me to say. @wochinge Any opinions/thoughts?

@dakshvar22
Copy link
Contributor

@joejuzl @wochinge Making a proposal to see if we can reach to a consensus quickly on the above question:

  1. New number of epochs get changed inside the meta parameter that is passed to the load method of all components.
  2. Add a boolean parameter to load method - finetune_mode which is set to True if the component is loaded in finetune mode.

What do you folks think?

@joejuzl
Copy link
Contributor

joejuzl commented Dec 3, 2020

@dakshvar22 So the solution we have come up with (today!) is as follows:

For NLU:

  • In Interpreter.load we pass in the new config along with the old model.
  • Interpreter.load updates the old metadata with the new epochs and calls Interpreter.create with a flag should_finetune.
  • Interpreter.create sets should_finetune in the context which gets passed to each component.
  • Trainer.__init__ now optionally takes the old loaded model and uses that pipeline e.g. self.pipeline = old_model.pipeline

@dakshvar22
Copy link
Contributor

@joejuzl Perfect! Small clarification:

Interpreter.create sets should_finetune in the context which gets passed to each component.

The context dictionary is what will be passed as part of the kwargs argument of load() methods of each component?

@joejuzl
Copy link
Contributor

joejuzl commented Dec 3, 2020

The context dictionary is what will be passed as part of the kwargs argument of load() methods of each component?

Yes exactly, via component_builder.load_component

@joejuzl
Copy link
Contributor

joejuzl commented Dec 3, 2020

For Core:

  • The new config is passed from Agent.load -> PolicyEnsemble.load.
  • Then the part of the new config for each policy is passed into its respective load which passes the new epochs into its constructor.
  • Still not clear exactly how the should_finetune flag will be passed around. I guess through the same path @wochinge ?

@wochinge
Copy link
Contributor Author

wochinge commented Dec 3, 2020

@dakshvar22 Our current approach would be to provide should_finetune through the constructor. The alternative would be to do so when calling train. Do you have preferences?

@dakshvar22
Copy link
Contributor

@wochinge Do you mean for Core specifically or NLU components also?
Either ways, we don't need it when calling train. It definitely has to be passed to the load() method of the ML components(DIETClassifier, TEDPolicy) so that the model weights can be instantiated in "training" mode and then load() would pass this parameter to the constructor anyways. If you want to see an example of what I mean, here is how I do it for ML components inside NLU.

@joejuzl joejuzl linked a pull request Dec 4, 2020 that will close this issue
4 tasks
joejuzl added a commit that referenced this issue Dec 7, 2020
joejuzl added a commit that referenced this issue Dec 9, 2020
* Load core model in fine-tuning mode

* Core finetune loading test

* Test and PR comments

* Fallback to default epochs

* Test policy and ensemble fine-tuning exception cases

* Remove epoch_override from Policy.load

* use kwargs

* fix

* fix train tests

* More test fixes

* Apply suggestions from code review

Co-authored-by: Daksh Varshneya <[email protected]>

* remove unneeded sklearn epochs

* Apply suggestions from code review

Co-authored-by: Tobias Wochinger <[email protected]>

* PR comments for warning strings

* Add typing

* add back invalid model tests

* small comments

Co-authored-by: Daksh Varshneya <[email protected]>
Co-authored-by: Tobias Wochinger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework type:enhancement ✨ Additions of new features or changes to existing ones, should be doable in a single PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants