Add tf and sklearn version info to policy metadata #1528
Conversation
rasa_core/policies/ensemble.py
Outdated
| @@ -97,12 +99,22 @@ def _create_action_fingerprints(training_events): | |||
| action_fingerprints[k] = {"slots": slots} | |||
| return action_fingerprints | |||
|
|
|||
| def _add_module_version_info(self, metadata: Dict[Text, Any]) -> None: | |||
| """Adds tensorflow and sklearn version info to metadata.""" | |||
| if any(isinstance(p, KerasPolicy) for p in self.policies): | |||
There was a problem hiding this comment.
I think instead of checking for the policy name, we could also just try to import, e.g.
try:
import tensorflow as tf
metadata["tensorflow"] = tf.__version__
except ImportError:
passWe could even make this more general, create a class variable versioned_packages = ["tensorflow", "sklearn", "rasa_core"] and then iterate over that:
import importlib
for package_name in self.versioned_packages:
p = importlib.import_module(package_name)
metadata[package_name] = p.__version__There was a problem hiding this comment.
Wouldn't we only want the version info in the metadata if it was actually used during training?
There was a problem hiding this comment.
This is a good point, this depends where the check for compatibility will be implemented. But I guess that is going to be in the policy used, so there shouldn't be an issue with persisting the verison even if tf is not used.
The reason why I don't want to use the KerasPolicy class check is, that e.g. the EmbeddingPolicy is using TF as well and is not inheriting from KerasPolicy so we would not persist the version.
There was a problem hiding this comment.
👍 In that case it's ready for another review
Co-Authored-By: ricwo <ric.wkr@gmail.com>
Proposed changes:
Status (please check what you already did):