Skip to content

Conversation

@ZQPei
Copy link
Contributor

@ZQPei ZQPei commented Dec 1, 2021

As discussed in issue #9617.

Recently tvm.micro is imported by tvmc even though USE_MICRO macro is OFF.
This bug was mistakenly introduced by #9229.

As we can see in

from . import micro

and
import tvm.micro.project as project
from tvm.micro.project import TemplateProjectError
from tvm.micro.project_api.client import ProjectAPIServerNotFoundError

I tried to fix it by,

  • adding a check_micro_support api to tvm.support,
  • lazy import tvm.micro after check_micro_support to preserve tvm with USE_MICRO OFF.

@ZQPei ZQPei force-pushed the fix_tvm_micro_not_supported_bug branch 2 times, most recently from d31fa50 to 99f4c72 Compare December 1, 2021 08:36
@ZQPei ZQPei force-pushed the fix_tvm_micro_not_supported_bug branch from 99f4c72 to f30f557 Compare December 1, 2021 08:38
@leandron leandron self-assigned this Dec 1, 2021
Copy link
Contributor

@leandron leandron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with this. Thanks for the fix.

Would like to also ask for opinions from @gromero, @areusch and @mehrdadh

@gromero
Copy link
Contributor

gromero commented Dec 2, 2021

@ZQPei oh pardon, even with @leandron tagging me I missed that PR. Thanks for reporting and proposing a fix.

Since I was not aware of yours I created earlier today #9632

I'm wondering if you could kindly test it too and check if it also fixes your issue.

My take is that tvmc micro parser should not be generated when USE_MICRO=OFF and also --device micro should be made unavailable for the user, saying why that's not supported.

@ZQPei
Copy link
Contributor Author

ZQPei commented Dec 2, 2021

I'm wondering if you could kindly test it too and check if it also fixes your issue.

My take is that tvmc micro parser should not be generated when USE_MICRO=OFF and also --device micro should be made unavailable for the user, saying why that's not supported.

Yeah, looks good to me. It can also fix the issue and is more concise.

@gromero
Copy link
Contributor

gromero commented Dec 2, 2021

I'm wondering if you could kindly test it too and check if it also fixes your issue.
My take is that tvmc micro parser should not be generated when USE_MICRO=OFF and also --device micro should be made unavailable for the user, saying why that's not supported.

Yeah, looks good to me. It can also fix the issue and is more concise.

@ZQPei 👍 Thanks for the review and for checking that it fixes your issue too.

@mehrdadh
Copy link
Member

mehrdadh commented Dec 2, 2021

@ZQPei if you're happy with the other PR, shall we close this PR and move forward with the other?

@ZQPei
Copy link
Contributor Author

ZQPei commented Dec 3, 2021

@ZQPei if you're happy with the other PR, shall we close this PR and move forward with the other?

Yes, of course

@ZQPei ZQPei closed this Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants