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

[RUNTIME] Add clear() function in tvm::Map class #7826

Merged
merged 1 commit into from
Apr 13, 2021

Conversation

Beya2019
Copy link
Contributor

std::map and std::unordered_map have the clear() member function to clear the contents.

tvm::Array also has the clear() member function.

In order to be consistent with tvm::Array/std::map/std::unordered_map, we add the clear() member function in tvm::Map class used existing function implementation

@tqchen @yzhliu Would you please have a look at this? Thanks very much.

@tqchen
Copy link
Member

tqchen commented Apr 12, 2021

cc @junrushao1994

@junrushao
Copy link
Member

@Beya2019 Thanks for the contribution! However, the change here may not be correct: never erase elements in the container when iterating them 🙂

@junrushao
Copy link
Member

The best way to clear a tvm::Map might be just re-creating one IMO

@Beya2019
Copy link
Contributor Author

The best way to clear a tvm::Map might be just re-creating one IMO

OK, according the suggestion, re-creating one call default constructor function: Map() { data_ = MapNode::Empty(); } , so clear function can call data_ = MapNode::Empty(); directly IMO

Do you think this is ok?

@junrushao
Copy link
Member

That looks good. Would you like to add some unittests? Thanks a lot!

@Beya2019
Copy link
Contributor Author

That looks good. Would you like to add some unittests? Thanks a lot!

OK. Add the unittests TEST(Map, Clear) in tests/cpp/container_test.cc. Please help review this submit again.Thanks a lot.

Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the contribution!

@junrushao junrushao merged commit ce86454 into apache:main Apr 13, 2021
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
Co-authored-by: honghua.cao <honghua.cao@streamcomputing.com>
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
Co-authored-by: honghua.cao <honghua.cao@streamcomputing.com>
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
Co-authored-by: honghua.cao <honghua.cao@streamcomputing.com>
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request May 11, 2021
Co-authored-by: honghua.cao <honghua.cao@streamcomputing.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants