(WIP) Triple classification #106
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…sec on fb15k, implemented an alternative way to find the thresholds, different slight changes
…which were not used in other implementations, include unique condition while sampling negatives from list to ensure same probability, Change find_thresholds so that the smallest score which gives the highest accuracy is used as threshold
…sary specifications in config files
…sholds, added comments for triple classification specification in default file, Included specification of evaluating on either test or valid data depending on the task (Test or validation during train)
…e_metrics, updated comment documentation, easier way to retrieve labels per relations in generate function, minor simplifications and error fixings
rgemulla
force-pushed
the
triple_classification
branch
from
34ec99c to
8a4416f
Compare
samuelbroscheit
commented
Jun 21, 2020
| @@ -11,8 +11,11 @@ lookup_embedder.dim: 100 | |||
| lookup_embedder.initialize: xavier_uniform_ | |||
| eval: | |||
| type: triple_classification | |||
| metrics_per.relation: False | |||
| triple_classification_random_seed: False | |||
| triple_classification.random_seed: False | |||
There was a problem hiding this comment.
triple_classification.random_seed is not used anymore
| @@ -423,6 +423,14 @@ valid: | |||
|
|
|||
| ## EVALUATION ################################################################## | |||
|
|
|||
| triple_classification: | |||
| random_seed: False | |||
There was a problem hiding this comment.
same here: random_seed can be removed
| @@ -31,6 +36,7 @@ def __init__(self, config: Config, configuration_key: str, dataset: Dataset): | |||
|
|
|||
| def _prepare(self,): | |||
| train_data = self.dataset.split("train") | |||
| #TODO probably outdated as it refers to out-commented code | |||
| @@ -20,7 +20,12 @@ def __init__(self, config: Config, configuration_key: str, dataset: Dataset): | |||
| self.o_entities = None | |||
| uni_sampler_config = config.clone() | |||
| # uni_sampler_config.set("negative_sampling.num_samples.s", self.get_option("num_samples.s")) | |||
| # TODO this is redundant as uniform.sample() is called with "num_samples" here in self.sample() | |||
There was a problem hiding this comment.
yes that is correct
Comment on lines
+25
to
+28
| # TODO maybe changing the API of KGEsampler.sample() to also accept a param "filter" | ||
| # as it is the case already with "num_samples" | ||
| # then we would not rely here on configuration options which actually | ||
| # belong to a training job |
There was a problem hiding this comment.
Absolutely. Currently the negative sampling config is mixing the Job and the Sampler. This should be split up in a separate PR.
| @@ -121,7 +141,8 @@ def _prepare(self): | |||
|
|
|||
| self.config.log("Generate data with corrupted and true triples...") | |||
|
|
|||
| if self.eval_split == "test": | |||
| # TODO maybe should be generalized to allow for other splits as valid_wo_unseen | |||
There was a problem hiding this comment.
Yes I agree, see my comment #115 (comment)
|
|
||
| positives_test = self.dataset.split("test") | ||
| negatives_test = self.dataset.split("test_negatives") | ||
| self.tune_data = torch.cat((positives_test, negatives_test)).to( |
| negatives_test = self.dataset.split("test_negatives") | ||
| self.tune_data = torch.cat((positives_test, negatives_test)).to( | ||
| self.device) | ||
| self.tune_labels = torch.cat( |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I didn't want this PR go to waste so I fixed it up to its current state. It works now except for the following, that would have to be adressed:
There is an incomplete attempt from me to integrate the TC datasets (where valid and test have a label) WN11 and FB13 into our dataset framework. The question here is, how we want to proceed here. My plan was to hae a
load_labels()function similar toload triples().