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

Support string sorting #44703

Open
tgsmith61591 opened this issue Nov 9, 2020 · 3 comments
Open

Support string sorting #44703

tgsmith61591 opened this issue Nov 9, 2020 · 3 comments
Assignees
Labels
comp:ops OPs related issues stat:awaiting tensorflower Status - Awaiting response from tensorflower type:feature Feature requests

Comments

@tgsmith61591
Copy link

Describe the feature and the current behavior/state.

Currently string sorting in the graph is unsupported:

Does not work:

tf.sort(['d', 'a', 'b'])
InvalidArgumentError: Value for attr 'T' of string is not in the list of allowed values: bfloat16, half, float, double, int8, int16, int32, int64, complex64, complex128
	; NodeDef: {{node Neg}}; Op<name=Neg; signature=x:T -> y:T; attr=T:type,allowed=[DT_BFLOAT16, DT_HALF, DT_FLOAT, DT_DOUBLE, DT_INT8, DT_INT16, DT_INT32, DT_INT64, DT_COMPLEX64, DT_COMPLEX128]> [Op:Neg]

Works (but not in the graph):

>>> x = tf.constant(['d', 'a', 'b'])
>>> tf.convert_to_tensor(sorted(x.numpy()), tf.string)
<tf.Tensor: shape=(3,), dtype=string, numpy=array([b'a', b'b', b'd'], dtype=object)>

Can tf.sort be amended to allow string sorting?

Will this change the current api? How?

No

Who will benefit with this feature?

Anyone who might need to string-sort in the graph

Any Other info.

@tgsmith61591 tgsmith61591 added the type:feature Feature requests label Nov 9, 2020
@Saduf2019 Saduf2019 added the comp:ops OPs related issues label Nov 10, 2020
@jvishnuvardhan jvishnuvardhan added the stat:awaiting tensorflower Status - Awaiting response from tensorflower label Nov 10, 2020
@ljwh
Copy link
Contributor

ljwh commented Nov 13, 2020

I checked the code of operation, the problem is that tf.sort actually call the TOPK OP that does not support tf.string input type but only real number type.

REGISTER_OP("TopKV2")
    .Input("input: T")
    .Input("k: int32")
    .Output("values: T")
    .Output("indices: int32")
    .Attr("sorted: bool = true")
    .Attr("T: realnumbertype")  // real num type only
    .SetShapeFn(TopKShapeFn);

If change of string type is acceptable with interface design and the scope of change ?
if it is acceptable, I'd like to commit the c++ code of impl.

@ljwh
Copy link
Contributor

ljwh commented Nov 27, 2020

@jvishnuvardhan

@preetham-salehundam
Copy link

I checked the code of operation, the problem is that tf.sort actually call the TOPK OP that does not support tf.string input type but only real number type.

REGISTER_OP("TopKV2")
    .Input("input: T")
    .Input("k: int32")
    .Output("values: T")
    .Output("indices: int32")
    .Attr("sorted: bool = true")
    .Attr("T: realnumbertype")  // real num type only
    .SetShapeFn(TopKShapeFn);

If change of string type is acceptable with interface design and the scope of change ? if it is acceptable, I'd like to commit the c++ code of impl.

May be have it included in tf.strings like

tf.strings.sort(['d', 'b', 'a'])
>> ['a', 'b', 'c']

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:ops OPs related issues stat:awaiting tensorflower Status - Awaiting response from tensorflower type:feature Feature requests
Projects
None yet
Development

No branches or pull requests

5 participants