Mousius commented on code in PR #10963:
URL: https://github.com/apache/tvm/pull/10963#discussion_r849743546
##########
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 could but what would be the achievement exactly with a unit
test for that internal function (_build_func_common)?
@gromero it's generally good practice to have comprehensive test coverage,
including input validation and error handling (which is essentially what these
`assert`s do). One reason is to figure out whether or not we can actually
trigger such a situation from a more public interface (i.e. test `LocalBuilder`
in some way to trigger this), another is to document the expected interface of
the function so it's well understood how we expect it to behave.
> More broadly, what has been our policy about testing assert in TVM code?
Should we test all the asserts? I recall there were a discussion some time ago
about it but could not find the PR, iirc you're discussing that with other Arm
folks.
There was a discussion awhile back on
https://github.com/apache/tvm/pull/9331#issuecomment-966241622, which was
resolved by @manupa-arm merging the PR without the tests added, no follow up
conversation has happened since. Different people follow the [Code Review
Guidelines](https://github.com/apache/tvm/blob/main/docs/contribute/code_review.rst#factors-to-consider-about-code-quality)
differently, and there are contributions without testing at all, so I think
the policy is very much down to the Committer rather than strictly defined.
--
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]
