[Target] Fix empty target and host for autotvm task

[zxybazh]

In order to patch the empty target and host situation in Issue 7790.

[comaniac]

Seems to me that __setstate__ also has this issue? If you save the state with both target and target_host are None, then you will get the same problem in L202.

[zxybazh]

Thanks @comaniac for pointing that out. I have added check in this case. In order to prevent this issue, I would take a look at other occurrences as well and update in this PR.

[comaniac]

Thanks @comaniac for pointing that out. I have added check in this case. In order to prevent this issue, I would take a look at other occurances as well and update in this PR.

Sure. Please comment here once it's ready for reivew.

[zxybazh]

Hi, I've checked all occurences and found other cases possible to reach the same error, e.g., the _build_module_no_factory function. Some the the functions calls does not have guarantee about whether target is None. Therefore, instead of adding extra checks in occurrences, I changed the check_and_update_host_consist function and add a warning to warn about some corner cases. After all IMO empty target should not be a typical case in the system, and should be avoided in the future. What do you think @comaniac ?

[zxybazh]

cc @junrushao1994

[comaniac]

Seems like we don't have a test case for check_and_update_host_consist. Should we add one?

[zxybazh]

Hi Cody, I don't intend to add test for the function because this is a temporary function for migration to the new target system. It should be deprecated soon after target host deprecation and is not encouraged for any other usage. I have also added the assertion as per your good point. Please review and let me know any other advice, thanks!

[comaniac]

Well...I don't quite agree on this point. Even this is just a temporary function, it is here now, and it is being used at more than 30 places in the current code base, which means a bug in this function affects 30+ places. Plus, the "tempoaray" could be one day or one to several months. It's not guaranteed that the final PR will be merged by when.

Would like to hear other's opinion.
cc @junrushao1994

[zxybazh]

Well, it does impact a lot of functions. I have added tests and fixed assertion warning as suggested. Please check, thanks!

[comaniac]

LGTM. Thanks

[junrushao]

Thanks for the fix!