Feature/skmeans

[g-roma]

This PR adds the spherical kmeans algorithm. Along with PR #98 it would enable feature learning using the algorithm by Coates and Ng. The client duplicates a lot of code from KMeans. I am not sure about how to best avoid this using the current client infrastructure, so I wonder if it can be improved in the future. Another alternative could be having both algorithms available from the same client, but I think this would lead to confusion, since the model learnt wth one algorithm is not applicable to the other.

[weefuzzy]

Can you add the SKMeans client itself? I might be able to make some suggestions about duplication.

Also, removing the PCA stuff from this branch would be tidy, but not critical.

[tremblap]

I'm removing James and Ted as reviewers for now for good old times nostalgia flavour

[g-roma]

Sorry added the client now.

[weefuzzy]

Are we sure about baking in the encoding scheme from Coates and Ng here?

I guess the argument in favour is that it enables recreating their feature learning scheme with the fewest objects. The arguments against would be that it's not strictly part of skmeans, but was a separate step used by C&N specifically in the feature learning setting, and they do discuss alternatives.

Obviously having it here doesn't preclude using an alternative scheme, because alpha can be set to 0. However, the other question is whether this encoder could be useful for other things if it were factored out, e.g. using NMF for feature learning?

[g-roma]

The soft thresholding function is similar to neural network activation functions, so NNFuncs is where it would fit best, but I don't think these functions would deserve their own client, so in practice I would still see this as part of the SKMeans client. So it would not help with code duplication. An interesting idea (maybe for the future?) could be to have a feature learning client that could use different learning techniques and encodings. For the moment, maybe it can be introduced as an option for skmeans. We can also use an MLP as autoencoder for feature learning.

[weefuzzy]

Ok, let's roll with what we have and see how we get on. I like the idea of some future feature learning object that could make it easy to explore options and manage some of the complexity / fiddliness.

[weefuzzy]

Should be OfflineIn OfflineOut

[weefuzzy]

The client duplicates a lot of code from KMeans. I am not sure about how to best avoid this using the current client infrastructure, so I wonder if it can be improved in the future.

They are indeed almost identical. Looking through a diff I see three points of difference

1. SKMeansClient mistakenly inheriting from AudioIn,ControlOut per my comment above
2. The insertion of the threshold parameter
3. A differeence in the API of the algorithms means that SKMeansClient calls SKMeans::transform and KMeansClient calls KMeans::getDistances

(1) should go away in any case
(2) I would prefer to go away in favour of decoupling encoding from clustering. If not, I can cook up something to insert into a ParameterDescriptorSet in a relatively painless way (for the caller 😄)
(3) I only noticed when I did the diff, and thinking towards the algorithms being their own API for wider use, I think this should change so that the interfaces match unless there's a strong reason for the difference.

In the absence of those differences, getting rid of the duplication is trivial with a template, and still pretty easy even if (2) and (3) remain, pending a way of composing the parameter descriptors.

[g-roma]

I agree that the embedding could be decoupled, I think I had planned to do it, then probably forgot. However, following other cases such as MLP objects, this would be used in the client, so it would not spare the threshold parameter, I am not sure I would make separate objects for encodings.

[tremblap]

ok it seems we have not resolve the code conflicts here, so we'll work on this on the nightlies post beta5 (hopefully) going out tomorrow.

[weefuzzy]

Suggested change

	out = _impl::asFluid(embedding);
	out <<= _impl::asFluid(embedding);

New copy assignment operator for FluidTensorView

[weefuzzy]

Suggested change

	src.row(0) =
	BufferAdaptor::ReadAccess(in.get()).samps(0, mAlgorithm.dims(), 0);
	src.row(0) <<=
	BufferAdaptor::ReadAccess(in.get()).samps(0, mAlgorithm.dims(), 0);

[weefuzzy]

Suggested change

	index rows = j.at("rows");
	index cols = j.at("cols");
	index rows = j.at("rows").get<index>();
	index cols = j.at("cols").get<index>();

The implicit conversion in at() is now not recommended by the JSON author, especially in from_json https://nicedoc.io/nlohmann/json#implicit-conversions

[weefuzzy]

Suggested change

	point =
	BufferAdaptor::ReadAccess(data.get()).samps(0, mAlgorithm.dims(), 0);
	point <<=
	BufferAdaptor::ReadAccess(data.get()).samps(0, mAlgorithm.dims(), 0);

[weefuzzy]

Suggested change

	point = BufferAdaptor::ReadAccess(get<kInputBuffer>().get())
	.samps(0, dims, 0);
	point <<= BufferAdaptor::ReadAccess(get<kInputBuffer>().get())
	.samps(0, dims, 0);

[weefuzzy]

Suggested change

	outBuf.allFrames()(Slice(0, 1), Slice(0, mAlgorithm.size())) = dest;
	outBuf.allFrames()(Slice(0, 1), Slice(0, mAlgorithm.size())) <<= dest;

[weefuzzy]

Suggested change

	double distance(Eigen::ArrayXd v1, Eigen::ArrayXd v2) const
	double distance(const Eigen::ArrayXd& v1, const Eigen::ArrayXd& v2) const

[weefuzzy]

I've had a quick look over for anything new that might fight back when you revisit this. Hopefully GH's new batch suggestion thing has worked for this. Mostly it's making sure we catch the change in assignment operator for FluidTensorView

[tremblap]

As I was learning how it works, and trying to write the doc, we noticed a discrepancy of the interface between KMeans and SKMeans. Luckily we have a solution.

transform are not returning the angular distance to the cluster, but the encoded activation. That has for effect that it is not just distance and the reverse polarity (large is good) to the equivalent methods in KMeans. and it confuses poor little learners like me.

solution: we change the method names to encode in this case

I'm happy to do it, here and in SC and in all the doc in progress if @g-roma agrees that I touch the code.

This solution is also good because it allows 2 more things:

• we could add transform to SKMeans later that returns the angular distance like our KMeans (and I reckon the scikit learn implementation)
• we could add encode to classic KMeans to return the dot products

This is a very small change, I can action now and we are still in time for release in beta 7 on Tuesday.

[g-roma]

I guess that since each algorithm does a different thing for transform I did not think you would expect these to the same or equievalent thing. In any case I am OK renaming transform to encode here.
Not sure what I would use for "encode" in classic k-means, but probably not dot product.

[tremblap]

cool if you don't mind I'll do it now/Monday first thing, and merge to dev so we don't miss the deadline of Monday night for beta7 Tuesday. I need to do it everywhere in the doc in progress too.

[tremblap]

ok all interface changes done. if nobody yells within the next two hours, I'll merge it in.