dgl-ke icon indicating copy to clipboard operation
dgl-ke copied to clipboard

Add default value of has_edge_importance

Open ryantd opened this issue 4 years ago • 3 comments

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 avatar Sep 01 '21 11:09 ryantd

Bump

ryantd avatar Sep 13 '21 09:09 ryantd

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.

classicsong avatar Sep 13 '21 09:09 classicsong

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

ryantd avatar Sep 14 '21 03:09 ryantd