Categorical search spaces #863
Conversation
| return tf.stack( | ||
| [tf.gather(tf.constant(self._tags[i]), row[i]) for i in range(len(row))] | ||
| ) |
There was a problem hiding this comment.
I think you can probably use tf.gather_nd instead of tf.gather and a for loop. Not sure, but you might be able to also get rid of tf.map_fn.
There was a problem hiding this comment.
Not sure that's possible, as self._tags is a non-rectangular Sequence[Sequence[str], not a tensor.
There was a problem hiding this comment.
Okay 👍 .
There might be a way (see example) to use ragged tensors with batch_dims arg to tf.gather, but that might not be more efficient than a for loop. Happy for you to ignore this :-)
There was a problem hiding this comment.
done a quick pass, looks ok to me, no major comments
I'll let @khurram-ghani approve it
|
|
||
| def __repr__(self) -> str: | ||
| """""" | ||
| return f"DiscreteSearchSpace({self._points!r})" |
There was a problem hiding this comment.
self.__class__.__name__ instead of a string?
There was a problem hiding this comment.
Looks good.
However, my personal preference is to not put the one_hot_encoder in the categorical and product search space classes. This could be a separate class that takes a search space as an __init__ argument.
I realise that these are just default implementations and users can always write separate implementations. As we know, there would be cases where we want to encode in different ways, especially for the product space (perhaps the categorical-only case is fine). I just think it is better to have the implied flexibility and not burn a default implementation in the spaces. I guess it just depends on our opinion on what is the most likely use case.
| encoded = tf.concat( | ||
| [encoder(column) for encoder, column in zip(encoders, columns)], axis=1 | ||
| ) |
There was a problem hiding this comment.
Given x is now a float type, do we need to do some checking/casting similar to to_tags? I'm not sure if tf.keras.layers.CategoryEncoding allows non-integer inputs.
There was a problem hiding this comment.
It's only the underling self.points in GeneralDiscreteSearchSpace that are now floats. The one hot encoding uses self.tags directly so isn't affected. (And this is all tested.)
There was a problem hiding this comment.
The x passed in by the user are the underlying indices, which can be float. We then call the encoder (after flattening) with these. From what I can see, the testing uses integer indices only, but I could have easily missed that. But in any case, it doesn't matter too much as I just tried out tf.keras.layers.CategoryEncoding with float types and it silently converts them into integers (in a weird way, see below). But perhaps we want to be explicit with our checking/assertions and not rely on tensorflow.
> e = tf.keras.layers.CategoryEncoding(4, output_mode="one_hot")
> e(tf.constant([3.0, 2.999999], dtype=tf.float32))
<tf.Tensor: shape=(2, 4), dtype=float32, numpy=
array([[0., 0., 0., 1.],
[0., 0., 1., 0.]], dtype=float32)>
> e(tf.constant([3.0, 2.9999999], dtype=tf.float32))
<tf.Tensor: shape=(2, 4), dtype=float32, numpy=
array([[0., 0., 0., 1.],
[0., 0., 0., 1.]], dtype=float32)>
Related issue(s)/PRs:
Summary
Add support for Categorical search spaces.
Modelling support will come in a future PR (see #862).
Fully backwards compatible: yes
PR checklist