Hardcode in error messages

balakkvj · July 29, 2020, 5:49pm

This is more of a suggestion than a question. I felt this was the most appropriate place to pitch it.
I’m using 0.4.3post2 version of DGL and I see some hardcoded messages in the feedback.
I suggest such messages should be removed.
Here’s an example:

/Users/xiangsx/work/dgl/dgl/src/graph/graph_serialize.cc:142: Check failed: fs: File name is not a valid local file name

zihao · July 29, 2020, 6:19pm

Hi, may I ask you what do you mean by hardcode?

balakkvj · July 29, 2020, 6:57pm

Hi,

By hardcode, I mean /Users/xiangsx/work/dgl/dgl/src/graph. Shouldn’t it be a relative path to graph_serialize.cc? It looks odd because if I wanted to edit the C++ code, it will look like I’ll have to use xiang's account

zihao · July 29, 2020, 7:05pm

That’s interesting, I have grep the repo and found such hardcode is used nowhere.

The error message you met comes from here:

github.com

dmlc/dgl/blob/9cd0d3f80d4773d776a888ba04dbd0561eec86d8/src/graph/serialize/dglgraph_serialize.cc#L71


DMLC_DECLARE_TRAITS(has_saveload, GraphDataObject, true);
}

namespace dgl {
namespace serialize {

bool SaveDGLGraphs(std::string filename, List<GraphData> graph_data,
                   std::vector<NamedTensor> labels_list) {
  auto fs = std::unique_ptr<SeekStream>(dynamic_cast<SeekStream *>(
    SeekStream::Create(filename.c_str(), "w", true)));
  CHECK(fs) << "File name " << filename << " is not a valid local file name";

  // Write DGL MetaData
  const uint64_t kVersion = 1;
  fs->Write(kDGLSerializeMagic);
  fs->Write(kVersion);
  fs->Write(GraphType::kImmutableGraph);
  fs->Seek(4096);

  // Write Graph Meta Data
  dgl_id_t num_graph = graph_data.size();

It’s interesting to find that Xiang’s name is attached to your error message.

balakkvj · July 29, 2020, 7:14pm

It happens to be in my libdgl.dylib!
Here’s a snippet from its output

strings /usr/local/lib/python3.7/site-packages/dgl/libdgl.dylib | grep xiang
/Users/xiangsx/work/dgl/dgl/src/array/array.cc
/Users/xiangsx/work/dgl/dgl/include/dgl/./runtime/ndarray.h
/Users/xiangsx/work/dgl/dgl/include/dgl/./runtime/packed_func.h
/Users/xiangsx/work/dgl/dgl/include/dgl/packed_func_ext.h
/Users/xiangsx/work/dgl/dgl/src/array/cpu/array_index_select.cc
/Users/xiangsx/work/dgl/dgl/src/array/cpu/array_op_impl.cc
/Users/xiangsx/work/dgl/dgl/src/array/cpu/array_pack.cc
/Users/xiangsx/work/dgl/dgl/src/array/cpu/array_repeat.cc
/Users/xiangsx/work/dgl/dgl/src/array/cpu/rowwise_sampling.cc
/Users/xiangsx/work/dgl/dgl/src/array/cpu/./rowwise_pick.h
/Users/xiangsx/work/dgl/dgl/src/array/cpu/spmat_op_impl.cc
/Users/xiangsx/work/dgl/dgl/src/array/cpu/spmat_op_impl_coo.cc
/Users/xiangsx/work/dgl/dgl/src/c_api_common.cc
/Users/xiangsx/work/dgl/dgl/src/kernel/binary_reduce.cc
/Users/xiangsx/work/dgl/dgl/include/dgl/graph_interface.h
/Users/xiangsx/work/dgl/dgl/include/dgl/base_heterograph.h
/Users/xiangsx/work/dgl/dgl/third_party/minigun/minigun/minigun/./cpu/advance.h
/Users/xiangsx/work/dgl/dgl/src/kernel/cpu/…/binary_reduce_impl.h
/Users/xiangsx/work/dgl/dgl/include/dgl/runtime/packed_func.h
/Users/xiangsx/work/dgl/dgl/src/random/cpu/choice.cc
/Users/xiangsx/work/dgl/dgl/include/dgl/random.h
/Users/xiangsx/work/dgl/dgl/src/random/random.cc
/Users/xiangsx/work/dgl/dgl/src/graph/network/msg_queue.cc
/Users/xiangsx/work/dgl/dgl/src/graph/network/socket_communicator.cc
/Users/xiangsx/work/dgl/dgl/src/graph/network/tcp_socket.cc
/Users/xiangsx/work/dgl/dgl/src/graph/nodeflow.cc
/Users/xiangsx/work/dgl/dgl/src/graph/pickle.cc
/Users/xiangsx/work/dgl/dgl/src/graph/sampler.cc
/Users/xiangsx/work/dgl/dgl/src/graph/sampler/metapath.cc
/Users/xiangsx/work/dgl/dgl/src/graph/sampler/randomwalk.cc
/Users/xiangsx/work/dgl/dgl/src/graph/sampling/neighbor/neighbor.cc

May be I should delete this dylib and reinstall afresh.

VoVAllen · July 31, 2020, 1:40pm

Hi, we’ve already improved the error message here in the latest master branch. Thanks for your suggestion!