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

Make sure HPO default lookup is based on class for losses #111

Merged
merged 3 commits into from
Oct 19, 2020

Conversation

cthoyt
Copy link
Member

@cthoyt cthoyt commented Oct 17, 2020

This PR uses the internal PyKEEN loss class hierarchy to make it possible to more elegantly define the HPO defaults, as in several other modules like models.

@codecov
Copy link

codecov bot commented Oct 19, 2020

Codecov Report

Merging #111 into master will decrease coverage by 0.06%.
The diff coverage is 54.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #111      +/-   ##
==========================================
- Coverage   69.43%   69.36%   -0.07%     
==========================================
  Files          91       91              
  Lines        5562     5566       +4     
  Branches      682      680       -2     
==========================================
- Hits         3862     3861       -1     
- Misses       1501     1506       +5     
  Partials      199      199              
Impacted Files Coverage Δ
src/pykeen/hpo/hpo.py 42.18% <16.66%> (-0.79%) ⬇️
src/pykeen/losses.py 98.66% <100.00%> (-0.02%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad15d9f...7e3dfce. Read the comment docs.

@cthoyt cthoyt requested review from mberr and mali-git October 19, 2020 11:06
@cthoyt cthoyt merged commit 9c189ec into master Oct 19, 2020
@cthoyt cthoyt deleted the improve-loss-hpo-defaults branch October 19, 2020 15:15
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