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

Make attribute support for std::vector<std::pair<int, int>> #3877

Closed
wanghaoshuang opened this issue Sep 5, 2017 · 2 comments · Fixed by #3878
Closed

Make attribute support for std::vector<std::pair<int, int>> #3877

wanghaoshuang opened this issue Sep 5, 2017 · 2 comments · Fixed by #3878
Assignees

Comments

@wanghaoshuang
Copy link
Contributor

wanghaoshuang commented Sep 5, 2017

Pad op need a attribute named paddings which is std::vector<std::pair<int, int>>.
So OperatorBase::AddAttr(...) should support for std::vector<std::pair<int, int>>.

@reyoung
Copy link
Collaborator

reyoung commented Sep 6, 2017

@wanghaoshuang @Canpio @qingqing01

I do not think this type of attribute is necessary since we could use four attributes in this solution, pad_left, pad_right, pad_top, pad_bottom. Moreover, if we add type, a list of int pairs, what about float pairs? string pairs? int tuples? (list<pair<float, float>>, list<pair<str, str>>, list<tuple<int, int, int>>)

If we adding list<pair<int, int>>, it is a trending to make that attribute more and more complex. I don't think it is a good choice.

@wanghaoshuang
Copy link
Contributor Author

wanghaoshuang commented Sep 8, 2017

@reyoung
pad_left, pad_right, pad_top, pad_bottom just suits 2-D tensor. Yes, we can use vector<int> instead of vector<pair<int, int>>. Which need developer writting python code to transfer list<tuple> to vector<int> and cpp code to transfer vector<int> to vector<pair<int, int>>.

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 a pull request may close this issue.

4 participants