awslabs / dgl-ke

High performance, easy-to-use, and scalable package for learning large-scale knowledge graph embeddings.
https://dglke.dgl.ai/doc/
Apache License 2.0
1.28k stars 196 forks source link

Add default value of has_edge_importance #227

Open ryantd opened 3 years ago

ryantd commented 3 years ago

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 commented 3 years ago

Bump

classicsong commented 3 years ago

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 commented 3 years ago

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.

https://github.com/awslabs/dgl-ke/blob/7b2975a6d5e507347340256caa1df1908e6a3d2e/python/dglke/models/general_models.py#L183-L212