ekalda commented on code in PR #13874:
URL: https://github.com/apache/tvm/pull/13874#discussion_r1093037582
##########
python/tvm/autotvm/tophub.py:
##########
@@ -110,6 +110,10 @@ def context(target, extra_files=None):
device = tgt.attrs.get("device", "")
if device != "":
possible_names.append(_alias(device))
+ # for cases when we do have explicitly defined -device in the target,
+ # we still might have information about it stored in keys container
+ # in other case we will load statistics for definitely irrelative stat
Review Comment:
Thanks for the detailed explanation :) No need to respond line by line, I
understand what it is doing and I think it is a really good change. However,
would you mind updating that comment in your PR? Otherwise the next person
reading it will be confused again. Reading that comment, I think it has the
logic inverted, I suppose you meant "For cases when we do NOT have explicitly
defined -device in the target"? In my opinion something along the lines "If we
don't have explicitly defined -device in the target, check target.keys for
device." would be enough. Or even better, explicitly list the order which we
use to determine the device:
1. target.device
2. target.keys
3. target.kind
In general, it is indeed pretty worrying that we can silently apply
completely irrelevant parameters from the tophub. I suppose in the ideal world
(out of scope for this PR) we'd have something like "Using tuning parameters
determined by tuning for < specific core or architecture >" printed to the
console during compilation, at least for tvmc, to warn the user about the
possibly bad choice of params.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
