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

Add default value of has_edge_importance #227

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ryantd
Copy link
Contributor

@ryantd ryantd commented Sep 1, 2021

There is an issue that will be raised in distributed training, like

Traceback (most recent call last):
  File "/usr/local/bin/dglke_server", line 33, in <module>
    sys.exit(load_entry_point('dglke==0.1.0.dev0', 'console_scripts', 'dglke_server')())
  File "/usr/local/lib/python3.6/site-packages/dglke-0.1.0.dev0-py3.6.egg/dglke/kvserver.py", line 178, in main
  File "/usr/local/lib/python3.6/site-packages/dglke-0.1.0.dev0-py3.6.egg/dglke/kvserver.py", line 159, in start_server
  File "/usr/local/lib/python3.6/site-packages/dglke-0.1.0.dev0-py3.6.egg/dglke/kvserver.py", line 120, in get_server_data
  File "/usr/local/lib/python3.6/site-packages/dglke-0.1.0.dev0-py3.6.egg/dglke/train_pytorch.py", line 100, in load_model
  File "/usr/local/lib/python3.6/site-packages/dglke-0.1.0.dev0-py3.6.egg/dglke/models/general_models.py", line 212, in __init__
AttributeError: 'Namespace' object has no attribute 'has_edge_importance'

Because the has_edge_importance argument is required for KEModel, kvserver.py and kvclient.py should have a default value of this, and then pass to the KEModel in dglke/models/general_models.py

@ryantd
Copy link
Contributor Author

ryantd commented Sep 13, 2021

Bump

@classicsong
Copy link
Contributor

classicsong commented Sep 13, 2021

Can you add an assertion as assert args.has_edge_importance == False in kvclient and kvserver, as this is a workaround. You did not implement has_edge_importance for distributed training.

@ryantd
Copy link
Contributor Author

ryantd commented Sep 14, 2021

Can you add an assertion as assert args.has_edge_importance == False in kvclient and kvserver

@classicsong May I have more explanations on this assertion? Why == False?

In my opinion, the distributed training example should be done as expected without any raised errors. And for KEModel class itself, it required the args.has_edge_importance passing from kvclient.py and kvserver.py. So I think the argument has_edge_importance in kvclient.py and kvserver.py, should be set explicitly by default.

class KEModel(object):
""" DGL Knowledge Embedding Model.
Parameters
----------
args:
Global configs.
model_name : str
Which KG model to use, including 'TransE_l1', 'TransE_l2', 'TransR',
'RESCAL', 'DistMult', 'ComplEx', 'RotatE', 'SimplE'
n_entities : int
Num of entities.
n_relations : int
Num of relations.
hidden_dim : int
Dimension size of embedding.
gamma : float
Gamma for score function.
double_entity_emb : bool
If True, entity embedding size will be 2 * hidden_dim.
Default: False
double_relation_emb : bool
If True, relation embedding size will be 2 * hidden_dim.
Default: False
"""
def __init__(self, args, model_name, n_entities, n_relations, hidden_dim, gamma,
double_entity_emb=False, double_relation_emb=False):
super(KEModel, self).__init__()
self.args = args
self.has_edge_importance = args.has_edge_importance

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