[tremblap]

fixes #120

I removed the nonsensical condition, and added 2 asserts to make sure we get the right values passed, assuming that a dataset with at least one point will have the right type. Maybe @weefuzzy will want to change the checking - I also thought of checking that the passed info was a vector of float instead of the asserts, and maybe that would be better. I'm opened to suggestions.

[tremblap]

example code to check in SC:

// make and plot a dataset with 5 globs
(
fork{defer{
	var counter = 0;
	var dict = Dictionary.newFrom(["cols",2,"data",Dictionary.new]);
	var xybuf = Buffer.alloc(s,2);

	1000.do{
		arg i;
		var x = [-0.7,0.7,0,0.7,-0.7][i % 5];
		var y = [-0.7,0.7,0,-0.7,0.7][i % 5];
		x = gauss(x,0.1).clip(-1,1);
		y = gauss(y,0.1).clip(-1,1);
		dict["data"][i] = [x,y];
	};

	~mean_seeds = FluidDataSet(s);

	~fp = FluidPlotter(bounds:Rect(0,0,800,800), xmin: -1, ymin: -1, dict:dict,mouseMoveAction:{
		arg view, x, y ...a;
		a[2] !? {
			xybuf.setn(0,[x,y]);
			~mean_seeds.addPoint(counter,xybuf,{
				counter = counter + 1;
				~mean_seeds.print;
			});
		};
	});
}}
)

// click to place at least 5 centroids, then run this code for kmean

(
~ds = FluidDataSet(s).load(~fp.dict);

~kmeans = FluidKMeans(s).setMeans(~mean_seeds);
~labels = FluidLabelSet(s);
~kmeans.predict(~ds,~labels,{
	~labels.dump({
		arg labels;
		~fp.categories_(labels);
	});
});
)

// same with skmean

(
~ds = FluidDataSet(s).load(~fp.dict);

~skmeans = FluidSKMeans(s).setMeans(~mean_seeds);
~labels = FluidLabelSet(s);
~skmeans.predict(~ds,~labels,{
	~labels.dump({
		arg labels;
		~fp.categories_(labels);
	});
});
)

[weefuzzy]

Presumably the numClusters parameter can now be incorrect after setMeans. So this method actually needs to update the params and return these.

Something like (untested)

  MessageResult<ParamValues> setMeans(InputDataSetClientRef srcClient)
  {
    auto srcPtr = srcClient.get().lock();
    if (!srcPtr) return Error(NoDataSet);
    auto dataSet = srcPtr->getDataSet();
    if (dataSet.size() == 0) return Error(EmptyDataSet);
    auto numClusters = dataSet.size(); 
    mAlgorithm.setMeans(dataSet.getData()); 
    // update params and return to host
    ParamValues newParams = mParams.get().toTuple();
    std::get<kNumClusters>(newParams) = numClusters; 
    return newParams; 
}

[weefuzzy]

ParamValues will need to be defined near the top of the class, like in the MLP objects:

using ParamValues = typename ParamSetViewType::ValueTuple;

[tremblap]

this is interesting, as the current public code does not behave the way I thought it would - changing the numCluster should always reset the algo to unFitted, but it currently doesn't.

could we fix that the same way? In effect, the expected behaviour for me:

• set does what I did here
• changing numCluster always invalidates the fitting (it doesn't now)

should i just put a separate bug report and a fix for that? Or am I just all confused?

[tremblap]

studying MLP code, I can see that the clients check if (!mAlgorithm.trained()) return Error(NoDataFitted); but i cannot find how it makes it 'untrained' when any parameter (like network structure) is changed. mTracker is only checked once in fit.

[tremblap]

an example of the misunderstood behaviour by yours truly:

----------begin_max5_patcher----------
1884.3oc0ZksbihCE8YmuBU7raWrujm546nyTtjAYa0AKwHDwISWc+sOZCLf
gDCMwS09ABn064bWzUR4GOrxZG8UToE3Qv2.qV8iGVsRUjrfUluWYcB9ZZNr
T0LqT5oSHB2ZstNN5UtpbmMfSvmQ.HvGTPwDNHq7Ih6FPACkgS4OBPLFksFP
nbvdLmiXOQ71He8Ih+FvYJ6YQGB1.xomQL.o5TZdUopYga.kbbdttQqAnWPD
.dOn7HsJOCrCApHpgLqVrxwDTJshnjsPSghgjVwyQbERrMkV.4oGwjCaYnTt
lIh7B2XuFjjHe5Fp9HNZiM3uuLRXR8.4XJDmoHB5tu+E2HKYY+7gGjOV+aRs
gVSE.9g9RY1KzU9Gm.aEPrmBBBVRDDLWD3p0AyBA9KIB7mMBzh9rPf2Rh.uo
if.OknG6OaD3tjHvc1Hv2Y1HvYIQfyjQfSTfTl8muir8b.vITYI7.5J.XBlq
Cv+OUH1aCinZAQWD+sBjFNVVMB9PpqjnMAxvVZ0ks1AxcLz5dMZcB+iCst9I
yFsAKIZurhaInwUSPAkBKYHGSIa6rpp6m.UjDqVp0a5Lg+mld2dI04N50y78
i0qMDNcj5rjHUj0DfyfXhPL+DrsMKcn8mmDLS9iyO1w2dt9wwyArDzYQeuBq
h7tqNU.xGFgtigPYmrV+AHM1jJiZwnfOPoNvZQNtKHPK.OYkxPPNBbBAIkfL
HGVh3OYsjJWCjcMIP3OYLGY0dvQLCPMHckvCLWrcFVoH9ZqQXkErnnUwqZ0E
I87cpZfhW2TDlnKJroHF5Ebc+CZJExDPjKvWESQ.VuFVmaqbXnYHFoBqDEcg
BEkQjTpDBT3kU.S0cVp4pqtEooSXzSQZdd5jVbBtvZBU9gbZ5ynrVo+HTMEH
Qfn1K4zo5LzdXUNe6dJgWh+WkD3HsEGn98FIbvJkXPI9+ECCya.vAFNiRjBQ
GMgr35oSDAOPaIzFLpVPfECzYgIffVFoxRAHqJ2AYRE0tbTKOTggNkl2spl9
ki1yMUWfIjdrHmVLdkL7giuSe2QEUd58FaUMkaqH5Z2JrI3aKguzks4v7bie
a2g+UHAeR3yxwZUfqcSkHBT.zikoLZddG7pq4kApISXimhNiy3GUSTaiAQyw
E0FQVMZ4L7ATIuaYb3gxtkTxeSS5sJpZmwGdKGcpHWfhtMP3cfK4kGomKMMr
1PqMAb43VZ6S2NHXmxe2cVzMhnNZXI.Bjq1I.Nj8FftWebLk.NEjSgY.wGT.
+HpNfI37Qb5QvY4ArrCAnULc.01yQmL+7aUwfajYzreBUNOQgs1LSXqMyLZ.
09AU0Y6spdsDSXpYRocVeoKiJ4wMhfcOq3pGAoz7xGaRNd7EYFYM1FshUGDO
.SoWuIIbiqLyBuqV14ZlxaXlJ9dxTGXzph6E4DnRgNL7iolQLhLYR+4yMhEz
DQ9yTdeyjctQNQZoHLXh8mMoDrnjhFGVyMdgATQIaRB7C78Rz+7cEYUnBd3M
mfGN1sEn5.psijq5.IC8ZqzfVD9PIae1l.arE+b7MjU3MRV1CSVg+uwUEPBJ
e1lNNpyPvWuyLyQv5D0i.1IhrKiYQx5y96nrLct5t2h0kt4hUGz4f+MQhH9t
dAtI9hIMzOxNbsnn.GuH2.+DamjP6nXgDoDqdBU2SypUExby6ibH4fIswnK4
9nyutiz3ttyityXAiVPYMoauIX.s3kjckoBLRJLJ8fr9gUukhTKRqMmMQfER
SGYQj1FGSZx8+a0qhoZ2MYcMUgH7FkgZg8SQHhuQgH3yTHBtU0gLB5HRwE6D
UFziXnzrALgGeXq8PdYeZCKxvzTQ7mNV1wR64XuDWGkOVjcRrs+0dUc7Gp86
LODV7cZqbgaUq2hIx8fipmKUaqenlgKLv5VzNSHjWuABoKr.Yp8Va+NpsNjS
uULde5Yf.PRgUG0Q47G4G3buD8tmF2GJ76NrWrCjFHr5pVLdzNQMUb5AFLCa
VdpSkciDpB+pnEioh7MQQNWYzX5oSioVn3Wz5Aea3t5V20n3Xg5X8fucUW6G
O1Kocs0oDX0f2lJaoJWbq3q8TmtEQ+zAmf0rRd8kKdpV.yWn5DzxRYIaFi5A
vRddFYa0mcyVHmyv6p35fYsO6tIcFCGxo6f48NXfgNBhGtDRcgN+aaUHNaUt
PfuHe+KpOb0end2Cbo7cvk81.L66W+z07mIcoGdK3AGuqZ+dD6W5KB.70R3o
hVW30Mdd46EaliKOvbEW8dfOzQQ0gZ2M8kDLoSPdIw997Jb1lmUmuyu.e8Lj
Iu4mRfyDg+GdUAdhTuU2as4JCBmLrcWbXaNuqe8A2307Acj4540oV35M8KHo
MlU8pa51ZJnedcFh3p74RZmN2Hox4dIStqn5acdhuSyi6MLOprzc9MmHG+6D
hbBtWST38Zhb07+6qjVpIx9dLQd2vDE0oMmvY5ybu9+gG0ox4Xq+uaHo4ieS
IK5VXfqUp53J8teQ4bz6dE6cmhWeehieWh8uGQUhUic+gO7yG9OP.s.FO
-----------end_max5_patcher-----------

[weefuzzy]

load and read have absolutely horrible special case handling in the host wrappers that happens to deal with updating the parameters. There's not any machinery for ad hoc messages that would return updated parameters (like setMeans).

[tremblap]

When I say it works, I mean that I have a debugger with the process and I go line for line, I see in all cases the mAlgo being changed, and the newParams too. the only diff is in the 'updated' client - one updates, the other not.

[weefuzzy]

Yes. It will 'work', but only up to the point where the host needs to actually store the updated parameters, which it won't do without a new overload being added.

[tremblap]

ok so should I put a bug report for load/read not working for kmeans, or will sorting this PR sort the other case too?

[weefuzzy]

load and read not working is its own issue, though I suspect it's because the default implementation for data clients doesn't return updated parameters (MLP overrides the default).