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

(WIP)Change embedder penalty API #101

Merged
merged 1 commit into from
May 25, 2020
Merged

(WIP)Change embedder penalty API #101

merged 1 commit into from
May 25, 2020

Conversation

Nzteb
Copy link
Contributor

@Nzteb Nzteb commented May 21, 2020

No description provided.

Copy link
Member

@rgemulla rgemulla left a comment

Choose a reason for hiding this comment

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

Looks good. Let's push this after PR #97

@@ -426,19 +426,32 @@ def append_num_parameter(job, trace):
job.post_epoch_trace_hooks.append(append_num_parameter)

def penalty(self, **kwargs) -> List[Tensor]:
# Note: If the subject and object embedder are identical, embeddings may be
# penalized twice. This is intended (and necessary, e.g., if the penalty is
# weighted).
if "batch" in kwargs and "triples" in kwargs["batch"]:
kwargs["batch"]["triples"] = kwargs["batch"]["triples"].to(
Copy link
Member

Choose a reason for hiding this comment

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

Store this in local variable triples for readability.

@rgemulla
Copy link
Member

Ok #97 is in, please rebase accordingly.

@Nzteb
Copy link
Contributor Author

Nzteb commented May 23, 2020

I have rebased and addressed the comment

@rgemulla rgemulla merged commit 952ec5a into uma-pi1:master May 25, 2020
@rgemulla
Copy link
Member

Thanks!

@Nzteb Nzteb deleted the reg_api branch May 30, 2020 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants