Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[USMP] Register hill climb algorithm #10182

Merged
merged 3 commits into from
Feb 9, 2022

Conversation

PhilippvK
Copy link
Contributor

While the hill-climbing algorithm for the Unified Static Memory Planner (see #8404, #9565) was recently added to TVM in #9704, it currently can not be used by the default TVM flow due to this error:

TVMError: Check failed: (algorithms.count(algo)) is false: The selected USMP algorithm : hill_climbis not defined. Please define it in the above algorithms map.

I was able to fix this by just adding an entry in the mentioned algorithms map.

If there was a reason whythe Hill Climb allocator was not registred until now, feel free to let me know.

@PhilippvK
Copy link
Contributor Author

CC @manupa-arm @d-smirnov

@manupak
Copy link
Contributor

manupak commented Feb 7, 2022

No reason, these two PRs went in parallely. In fact, I discussed with @d-smirnov today of doing this today.

@PhilippvK
Copy link
Contributor Author

@manupa-arm Thank you for your fast response. If you would like be to add anything alse to this mini PR or if your would perfer to add the changes yourself, feels free to do so.

I think the CI job crashed due to an issue which is not related to this PR. Should I retrigger it with a new push?

@manupak
Copy link
Contributor

manupak commented Feb 8, 2022

Hi @PhilippvK ,

Once again, thanks for picking this up :)

Only thing I could think of enabling this in the test here (which should be easy enough to do) :

@pytest.mark.parametrize(
"model_url, usmp_algo, workspace_size,",
[
(MOBILENET_V1_URL, "greedy_by_size", 4845696),
(MOBILENET_V1_URL, "greedy_by_conflicts", 4444288),
],
)

and yes... please retrigger.

@PhilippvK
Copy link
Contributor Author

That sounds like a good idea! I will give it a try.

Copy link
Contributor

@manupak manupak left a comment

Choose a reason for hiding this comment

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

LGTM!

@manupak manupak merged commit 1571112 into apache:main Feb 9, 2022
@manupak
Copy link
Contributor

manupak commented Feb 9, 2022

Thanks @PhilippvK !

ylc pushed a commit to ylc/tvm that referenced this pull request Feb 16, 2022
* USMP: Register hill_climb algo in algorithms map

* USMP: Add missing space to assertion message

* USMP: Integrate hill_climb algo in test_crt_aot_usmp
@PhilippvK PhilippvK deleted the register-hill-climb branch January 23, 2024 16:59
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.

2 participants