-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
7330 Add functionality to check if a model is fine-tunable #7427
7330 Add functionality to check if a model is fine-tunable #7427
Conversation
b365fb3
to
60e0ac1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good 👍 Please add types for pytests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
production code looks great! Need to have another look at the tests later 👍
tests/core/test_model.py
Outdated
(_fingerprint(stories=["other"]), False), | ||
], | ||
) | ||
def test_fine_tune_fingerprint_changed(fingerprint2, changed): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
types please
tests/core/test_model.py
Outdated
[("2.1.0", "2.1.0", True), ("2.0.0", "2.1.0", True), ("2.1.0", "2.0.0", False),], | ||
) | ||
async def test_can_fine_tune_min_version( | ||
project: Text, monkeypatch, old_model_version, min_compatible_version, can_tune |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
types please
tests/core/test_model.py
Outdated
if "epochs" in p: | ||
p["epochs"] += 10 | ||
|
||
importer.get_config = asyncio.coroutine(lambda: config) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
asyncio.coroutine
is deprecated 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👍
@dakshvar22 is the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Proposed changes:
Adds a new method
rasa.model.can_fine_tune
which uses fingerprinting to determine if a model can be fine-tuned.Status (please check what you already did):
black
(please check Readme for instructions)