Mousius commented on code in PR #10963:
URL: https://github.com/apache/tvm/pull/10963#discussion_r876945353
##########
python/tvm/autotvm/measure/measure_methods.py:
##########
@@ -496,7 +497,8 @@ def set_task(self, task):
def _build_func_common(measure_input, runtime=None, check_gpu=None,
build_option=None):
"""Common part for building a configuration"""
target, task, config = measure_input
- target, task.target_host = Target.check_and_update_host_consist(target,
task.target_host)
+ assert not isinstance(target, (Map, dict)), "It's expected that 'target'
is a string here."
Review Comment:
I think we agree that the `assert` should be in the `Target` code, I totally
agree that it's outside of the scope of this PR.
If we introduce an `assert` in this PR, when does that get cleaned up? If
it's an immediate follow up PR, then we can either not introduce it in this PR
or we can put it anywhere as it'll get removed straight away. If not, we've
left this in an untested state, with an extra `assert` for a different
component which may never get cleaned up.
At a cursory glance I think `check_and_update_host_consist` supports any of
the valid `Target` formats? So I'd suggest a follow up clarifying that with
test cases.
In the spirit of getting this merged rather than continuing to debate that
function here, I would go without the `assert` - what do you think?
--
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]
