hidet-org / hidet

An open-source efficient deep learning framework/compiler, written in python.
https://hidet.org
Apache License 2.0
656 stars 53 forks source link

[Fixbug] Fix graph metadata hash #428

Closed KTong821 closed 9 months ago

KTong821 commented 9 months ago

Same graphs should have the same hash. Graph node traversal used in generating hash is non-deterministic, leading to different hashes for the same graph.

This can prevent graph runs from using the fast path, since dispatch tables cannot be found for the current (different) hash!

yaoyaoding commented 9 months ago

Very interesting!

Is there any example to reproduce the behavior of producing different hashes?

KTong821 commented 9 months ago

@yaoyaoding I am able to observe this on my resnet app branch, with >30 layers. Was not able to observe on smaller graphs.

graph_analyze in graph_impl.py runs a topological sort using iteration over unordered data structure (set, dict), causing this behaviour.

vadiklyutiy commented 9 months ago

@KTong821 What is the reason of Graph node traversal used in generating hash is non-deterministic?

yaoyaoding commented 9 months ago

using iteration over unordered data structure (set, dict), causing this behaviour.

Good catch! We might need to avoid iterating set when we construct the topological order of nodes of flow graph. Using dict is fine because "Dictionaries preserve insertion order." from https://docs.python.org/3/library/stdtypes.html#mapping-types-dict

@vadiklyutiy for your question, we used set and iterate it when we compute a topological order of our computation graph.

KTong821 commented 9 months ago

@yaoyaoding yes, making the topological ordering one-to-one is a better solution than sorting the hash input string. We might make different graphs have the same hash by sorting here. Will update PR.

yaoyaoding commented 9 months ago

Thanks! @KTong821