| --- Log opened Fri May 11 00:00:01 2018 | ||
| -!- travis-ci [~travis-ci@ec2-54-224-238-97.compute-1.amazonaws.com] has joined #shogun | 00:40 | |
| travis-ci | it's Shubham Shukla's turn to pay the next round of drinks for the massacre he caused in shubham808/shogun: https://travis-ci.org/shubham808/shogun/builds/377448446 | 00:40 |
|---|---|---|
| -!- travis-ci [~travis-ci@ec2-54-224-238-97.compute-1.amazonaws.com] has left #shogun [] | 00:40 | |
| -!- travis-ci [~travis-ci@ec2-54-224-238-97.compute-1.amazonaws.com] has joined #shogun | 00:46 | |
| travis-ci | it's Shubham Shukla's turn to pay the next round of drinks for the massacre he caused in shubham808/shogun: https://travis-ci.org/shubham808/shogun/builds/377448446 | 00:46 |
| -!- travis-ci [~travis-ci@ec2-54-224-238-97.compute-1.amazonaws.com] has left #shogun [] | 00:46 | |
| -!- HeikoS [~heiko@host81-153-166-104.range81-153.btcentralplus.com] has joined #shogun | 01:24 | |
| -!- mode/#shogun [+o HeikoS] by ChanServ | 01:24 | |
| -!- HeikoS [~heiko@host81-153-166-104.range81-153.btcentralplus.com] has quit [Client Quit] | 01:24 | |
| -!- Chris____ [8602a493@gateway/web/freenode/ip.134.2.164.147] has quit [Ping timeout: 260 seconds] | 07:47 | |
| -!- Chris___ [8602b65f@gateway/web/freenode/ip.134.2.182.95] has joined #shogun | 08:13 | |
| -!- shubham808 [dfb0b96c@gateway/web/freenode/ip.223.176.185.108] has joined #shogun | 08:44 | |
| -!- shubham808 [dfb0b96c@gateway/web/freenode/ip.223.176.185.108] has quit [Ping timeout: 260 seconds] | 09:21 | |
| -!- shubham808 [dfb0b96c@gateway/web/freenode/ip.223.176.185.108] has joined #shogun | 10:36 | |
| -!- shubham808 [dfb0b96c@gateway/web/freenode/ip.223.176.185.108] has quit [Ping timeout: 260 seconds] | 10:41 | |
| Chris___ | Hi wiking, are you there? | 10:48 |
| @wiking | yoyo | 10:48 |
| @wiking | yes yes | 10:48 |
| @wiking | here | 10:48 |
| @wiking | shoot | 10:48 |
| sukey1 | [https://github.com/shogun-toolbox/shogun] New branch feature/transformers created | 10:50 |
| @wiking | wuwei, i've created ^ this for you | 10:50 |
| @wiking | any preprocessor&converter changes should go into that branch | 10:50 |
| Chris___ | I'm using nested cross validation (the inner one is done using CrossValidation), the outer one is done by hand since I don't know how to it with the CrossValidation function as well | 10:51 |
| @wiking | oh | 10:52 |
| Chris___ | the problem is when I'm plotting the ROC values of the CrossValidation function using ParameterObserver I get a nice and smooth ROC curve | 10:52 |
| Chris___ | when I fetch the values from the outer CV (without CrossValidation and ParameterObserver) it's not "smooth" | 10:52 |
| @wiking | mmm | 10:53 |
| @wiking | so when you do the outer CV | 10:53 |
| @wiking | what exactly do you do | 10:53 |
| @wiking | you yourself do the splitting | 10:53 |
| @wiking | and then train/test? | 10:53 |
| Chris___ | yes I split it, use the test set for the inner CV and hyperparameter selection | 10:54 |
| Chris___ | the best parameters are then used to evaluate the outer test set | 10:54 |
| @wiking | mmm i'm not so sure if i get exactly why do you do this :) | 10:55 |
| Chris___ | so in my case this is done 5 times, for each evaluation on the test set I save the roc values and compute the mean which is then used for plotting | 10:55 |
| @wiking | i mean i dont get why there's this double cv going on? :) | 10:57 |
| @wiking | or it's not a double cv | 10:57 |
| @wiking | you just simply do once 'manually' your CV | 10:58 |
| @wiking | and once with shogun's cv? | 10:58 |
| Chris___ | it's a nested CV | 10:58 |
| Chris___ | 5-fold outer CV on the whole dataset, 5-fold inner CV on the respective training set for hyperparameter optimization | 10:59 |
| @wiking | ok i'm not sure if i get what this brings on your table :) | 10:59 |
| @wiking | i mean nestedness :) | 10:59 |
| @wiking | ah i see | 11:00 |
| @wiking | http://jmlr.csail.mit.edu/papers/volume11/cawley10a/cawley10a.pdf | 11:00 |
| @wiking | ? | 11:00 |
| Chris___ | I didn't know this paper but I guess they argue why you should use nested CV?! :) | 11:03 |
| @wiking | :> | 11:13 |
| @wiking | yep | 11:13 |
| @wiking | it's about that | 11:13 |
| -!- shubham808 [dfb0bfb4@gateway/web/freenode/ip.223.176.191.180] has joined #shogun | 11:15 | |
| Chris___ | so when I use the CrossValidation function with ParameterObserver I iterate over all folds for each observation and get the ROC values for each fold, when I plot this it generates a nice curve | 11:15 |
| Chris___ | but somehow this is not possible when I just evaluate each fold "manually" and plot the corresponding ROC values | 11:16 |
| -!- HeikoS [~heiko@host86-146-49-221.range86-146.btcentralplus.com] has joined #shogun | 11:34 | |
| -!- mode/#shogun [+o HeikoS] by ChanServ | 11:34 | |
| -!- travis-ci [~travis-ci@ec2-54-160-138-195.compute-1.amazonaws.com] has joined #shogun | 11:40 | |
| travis-ci | it's Heiko Strathmann's turn to pay the next round of drinks for the massacre he caused in shogun-toolbox/shogun: https://travis-ci.org/shogun-toolbox/shogun/builds/377637192 | 11:40 |
| -!- travis-ci [~travis-ci@ec2-54-160-138-195.compute-1.amazonaws.com] has left #shogun [] | 11:40 | |
| -!- travis-ci [~travis-ci@ec2-184-73-70-8.compute-1.amazonaws.com] has joined #shogun | 11:51 | |
| travis-ci | it's Heiko Strathmann's turn to pay the next round of drinks for the massacre he caused in shogun-toolbox/shogun: https://travis-ci.org/shogun-toolbox/shogun/builds/377637192 | 11:51 |
| -!- travis-ci [~travis-ci@ec2-184-73-70-8.compute-1.amazonaws.com] has left #shogun [] | 11:51 | |
| -!- shubham808 [dfb0bfb4@gateway/web/freenode/ip.223.176.191.180] has quit [Ping timeout: 260 seconds] | 11:52 | |
| -!- shubham808 [dfb0bfb4@gateway/web/freenode/ip.223.176.191.180] has joined #shogun | 11:55 | |
| -!- shubham808 [dfb0bfb4@gateway/web/freenode/ip.223.176.191.180] has quit [Ping timeout: 260 seconds] | 12:25 | |
| sukey1 | [https://github.com/shogun-toolbox/shogun] Pull Request https://github.com/shogun-toolbox/shogun/pull/4280 synchronized by shubham808 | 12:57 |
| sukey1 | [https://github.com/shogun-toolbox/shogun] Pull Request https://github.com/shogun-toolbox/shogun/pull/4280 synchronized by shubham808 | 13:00 |
| -!- shubham808 [dfb0bfb4@gateway/web/freenode/ip.223.176.191.180] has joined #shogun | 13:04 | |
| -!- iglesiasg [6cab80bd@gateway/web/freenode/ip.108.171.128.189] has joined #shogun | 13:05 | |
| -!- HeikoS [~heiko@host86-146-49-221.range86-146.btcentralplus.com] has quit [Ping timeout: 240 seconds] | 13:11 | |
| -!- HeikoS [~heiko@host86-146-49-221.range86-146.btcentralplus.com] has joined #shogun | 13:11 | |
| -!- mode/#shogun [+o HeikoS] by ChanServ | 13:11 | |
| iglesiasg | HeikoS: hallo | 13:13 |
| @HeikoS | lala | 13:13 |
| iglesiasg | is there a way to see timeline of the updates to any wiki page? | 13:13 |
| iglesiasg | if for example I would like to see if any new page has been added or if there was a recent update to any page | 13:14 |
| -!- shubham808 [dfb0bfb4@gateway/web/freenode/ip.223.176.191.180] has quit [Ping timeout: 260 seconds] | 13:30 | |
| @HeikoS | iglesiasg: yes | 13:32 |
| @HeikoS | there is | 13:32 |
| @HeikoS | but only per page I think | 13:32 |
| @HeikoS | otherwise dont know | 13:33 |
| @HeikoS | but would be good to have actually | 13:33 |
| -!- travis-ci [~travis-ci@ec2-184-73-70-8.compute-1.amazonaws.com] has joined #shogun | 13:48 | |
| travis-ci | it's Shubham Shukla's turn to pay the next round of drinks for the massacre he caused in shubham808/shogun: https://travis-ci.org/shubham808/shogun/builds/377679026 | 13:48 |
| -!- travis-ci [~travis-ci@ec2-184-73-70-8.compute-1.amazonaws.com] has left #shogun [] | 13:48 | |
| -!- shubham808 [6adb02b3@gateway/web/freenode/ip.106.219.2.179] has joined #shogun | 13:59 | |
| -!- shubham808 [6adb02b3@gateway/web/freenode/ip.106.219.2.179] has quit [Ping timeout: 260 seconds] | 14:08 | |
| -!- travis-ci [~travis-ci@ec2-54-160-138-195.compute-1.amazonaws.com] has joined #shogun | 14:13 | |
| travis-ci | it's Shubham Shukla's turn to pay the next round of drinks for the massacre he caused in shubham808/shogun: https://travis-ci.org/shubham808/shogun/builds/377679856 | 14:13 |
| -!- travis-ci [~travis-ci@ec2-54-160-138-195.compute-1.amazonaws.com] has left #shogun [] | 14:13 | |
| -!- HeikoS [~heiko@host86-146-49-221.range86-146.btcentralplus.com] has quit [Ping timeout: 268 seconds] | 14:16 | |
| iglesiasg | yeah per page of course, it's quite visible | 14:19 |
| -!- iglesiasg [6cab80bd@gateway/web/freenode/ip.108.171.128.189] has quit [Quit: Page closed] | 14:20 | |
| -!- travis-ci [~travis-ci@ec2-54-160-138-195.compute-1.amazonaws.com] has joined #shogun | 14:53 | |
| travis-ci | it's Shubham Shukla's turn to pay the next round of drinks for the massacre he caused in shubham808/shogun: https://travis-ci.org/shubham808/shogun/builds/377679856 | 14:53 |
| -!- travis-ci [~travis-ci@ec2-54-160-138-195.compute-1.amazonaws.com] has left #shogun [] | 14:53 | |
| -!- shogun-buildbot [~shogun-bu@7nn.de] has quit [Remote host closed the connection] | 15:21 | |
| -!- shogun-buildbot [~shogun-bu@7nn.de] has joined #shogun | 15:21 | |
| @wiking | shogun-buildbot: force build --branch=develop 'bionic - libshogun' | 15:21 |
| -!- shubham808 [dfbda541@gateway/web/freenode/ip.223.189.165.65] has joined #shogun | 16:26 | |
| -!- Chris___ [8602b65f@gateway/web/freenode/ip.134.2.182.95] has quit [Ping timeout: 260 seconds] | 16:52 | |
| -!- HeikoS [~heiko@untrust-out.swc.ucl.ac.uk] has joined #shogun | 16:55 | |
| -!- mode/#shogun [+o HeikoS] by ChanServ | 16:55 | |
| sukey1 | [https://github.com/shogun-toolbox/shogun] Pull Request https://github.com/shogun-toolbox/shogun/pull/4280 merged by karlnapf | 17:18 |
| sukey1 | [https://github.com/shogun-toolbox/shogun] New commit https://github.com/shogun-toolbox/shogun/commit/986f97ee9d25898b8a3f75fa9976cc55305b4fdc by karlnapf | 17:18 |
| @wiking | HeikoS, why did you merge it? | 17:18 |
| @wiking | HeikoS, it would have been good to have geektoni's comments fixed | 17:19 |
| @wiking | as not all of them were actually fixed | 17:19 |
| @HeikoS | which one is not? | 17:20 |
| @wiking | the ones that are not shown | 17:20 |
| @wiking | outdated | 17:20 |
| @HeikoS | I think they are all addressed | 17:22 |
| @wiking | mmm then i guess there's a bug in github? | 17:22 |
| @HeikoS | wiking: btw | 17:22 |
| @wiking | as those comments should be hidden | 17:22 |
| @HeikoS | we ran into a design question | 17:22 |
| @HeikoS | about constness and some | 17:22 |
| @wiking | in what context? | 17:22 |
| @HeikoS | turning const pointers into some | 17:23 |
| @HeikoS | like shared_ptr<const bla*> | 17:23 |
| @HeikoS | lisitsyn argued that this generates too many types | 17:23 |
| @HeikoS | and suggested to move the const for objects into runtime | 17:23 |
| @HeikoS | i.e. a flag in some | 17:23 |
| @wiking | miju | 17:24 |
| @wiking | i mean this is a bit like | 17:24 |
| @wiking | "lets not use compiler :) | 17:24 |
| @wiking | because we want interpeter | 17:24 |
| @wiking | :) | 17:24 |
| @HeikoS | indeed | 17:24 |
| @HeikoS | I know you wont like it | 17:24 |
| @wiking | i.e. i wouldn't throw out compiler features | 17:24 |
| @HeikoS | but the type thing is true as well | 17:24 |
| @wiking | i mean when there's a const | 17:24 |
| @HeikoS | it is a general problem of shared ptr concept | 17:25 |
| @wiking | compiler can do you many tricks | 17:25 |
| @wiking | when optimizing the codew | 17:25 |
| @wiking | now if you hide this | 17:25 |
| @wiking | then you are a bit playing against yourself | 17:25 |
| @HeikoS | lisitsyn said that one doesnt really see shared_ptr<const X*> in code | 17:25 |
| @HeikoS | but I cannot judge this | 17:25 |
| @HeikoS | yeah I agree with you | 17:25 |
| @wiking | when are you passsing though | 17:25 |
| @HeikoS | but the other solution is also shitty | 17:25 |
| @wiking | to shared_ptr a const? | 17:25 |
| lisitsyn | hey | 17:25 |
| @wiking | *to | 17:25 |
| @HeikoS | lisitsyn: ah there we are | 17:26 |
| @wiking | HeikoS, what i mean is some is about swig | 17:26 |
| @wiking | you can use shared_ptr internally | 17:26 |
| @HeikoS | wiking: shared_ptr has the same problem | 17:26 |
| @wiking | and there constness is sovled | 17:26 |
| @wiking | or? | 17:26 |
| @wiking | you cannot pass a const A* to std::shared_ptR? | 17:26 |
| @wiking | or what? | 17:27 |
| lisitsyn | I am not sure compiler things are that related to swig | 17:27 |
| @wiking | no they are not | 17:28 |
| lisitsyn | I mean you can't optimize much when you pass around with Pysomething | 17:28 |
| @wiking | but i mean | 17:28 |
| @wiking | if you start using Some<A> and have a flag | 17:28 |
| @wiking | for constness | 17:28 |
| @wiking | then the problem is that where do you have the clear cut? | 17:28 |
| @wiking | meaning that somebody doesn't start to use within the lib | 17:28 |
| @wiking | the same concept | 17:28 |
| @wiking | :( | 17:28 |
| @wiking | HeikoS, what's with shared_ptr and const A*? | 17:29 |
| lisitsyn | yeah this concept is a bit tricky | 17:29 |
| @wiking | i'm not so sure if i udnerstand there the problem | 17:29 |
| lisitsyn | but I think we either don't use consts | 17:29 |
| lisitsyn | or we trick it somehow because otherwise we get combinations | 17:29 |
| lisitsyn | like const vs non-const etc | 17:29 |
| @HeikoS | so I ran into this when converting labels, a few places in shogun have const pointers for labels, so then there suddenly have to be two methods for that | 17:31 |
| @HeikoS | doable, but as lisitsyn says, there is a combinatorial expansion in type combinations | 17:32 |
| lisitsyn | yeah I am afraid we'd have to double everything | 17:32 |
| @wiking | HeikoS, code? | 17:34 |
| @wiking | because i still dot really get the problem with shared_ptr | 17:34 |
| lisitsyn | shared_ptr<T> and shared_ptr<const T> are two different types | 17:34 |
| lisitsyn | that's the problem we have | 17:34 |
| @wiking | yeah but you would have that problem anyways | 17:35 |
| lisitsyn | no if you don't use const :D | 17:35 |
| @wiking | yeah | 17:35 |
| @wiking | :) | 17:35 |
| @wiking | but i mean it's not coming from shared_ptr per se | 17:35 |
| @wiking | :) | 17:35 |
| lisitsyn | from pointers also | 17:36 |
| lisitsyn | it's just the same thing if you use pointers | 17:36 |
| @wiking | but i mean you can create a generator for this in the worst case no? | 17:37 |
| @wiking | it's shitty | 17:37 |
| lisitsyn | what's generator? | 17:37 |
| @wiking | i mean that you have a geneator macro | 17:37 |
| @wiking | that creates both declaration | 17:37 |
| @wiking | const-ed and non-const | 17:38 |
| lisitsyn | but if they are just the same | 17:38 |
| lisitsyn | what's the point of using const then | 17:38 |
| @wiking | yeah dunno | 17:40 |
| @wiking | i feel that this is shitty :) | 17:40 |
| @wiking | no matter how you touch it | 17:40 |
| @HeikoS | I am currently blocked because some of shogun's code uses const poitners for labels and features (as it should!) but I cannot do the dynamic checking what type of labels it is without copy-pasting the method | 17:42 |
| @HeikoS | right now, most of shogun doesnt use const at all | 17:43 |
| @HeikoS | for objects that is | 17:43 |
| @HeikoS | only few more recent additions do that | 17:43 |
| @HeikoS | thats why the problem only appeared now | 17:43 |
| @wiking | mmm | 17:44 |
| @wiking | i guess this is a dispatch story? | 17:44 |
| @wiking | or? | 17:45 |
| @wiking | hence my question whether you can point me to a code | 17:45 |
| @wiking | or not | 17:45 |
| @HeikoS | it came up in the dispatching for labels yes | 17:45 |
| @HeikoS | e.g. binary_labels | 17:45 |
| @HeikoS | in CBinaryLabels.h | 17:45 |
| @wiking | yeah this is the same story | 17:45 |
| @wiking | of what i've mentioned previously | 17:46 |
| @wiking | that actually these type of dispatchers | 17:46 |
| @HeikoS | called from a method where I have a const pointer | 17:46 |
| @HeikoS | this is internal code btw, not swig | 17:46 |
| @wiking | yeah | 17:50 |
| @wiking | but then you can do | 17:50 |
| @wiking | binary_label(THE CORRECT TYPE a) | 17:50 |
| @wiking | right? | 17:50 |
| @wiking | and then then compiler will choose the right thing for you | 17:50 |
| @wiking | wouldnt it? | 17:50 |
| @wiking | swig then of course would autogen the dispatcher | 17:50 |
| @wiking | as usual | 17:50 |
| @HeikoS | swig doesnt see this | 17:50 |
| @wiking | then even better :L) | 17:50 |
| @HeikoS | I could have one | 17:50 |
| @HeikoS | Some<const BinaryLabels> binary_labels(const Labels*) | 17:50 |
| @HeikoS | and one | 17:50 |
| @HeikoS | Some<CBinaryLabels> binary_labels(CLabels*) | 17:50 |
| @HeikoS | yes | 17:50 |
| @wiking | but | 17:50 |
| @wiking | i wonder why some? | 17:50 |
| @wiking | :) | 17:50 |
| @wiking | if it's internal | 17:50 |
| @wiking | i mean this is not crucial question of course | 17:50 |
| @wiking | but if you never wanna expose this to swig then you dont have to use some... | 17:50 |
| @HeikoS | dont we want to switch to some auto-ref counting internally as well? | 17:50 |
| @HeikoS | but that aside, you are right, no need to use Some | 17:50 |
| @wiking | anyhow | 17:50 |
| @wiking | that doesn't matter | 17:50 |
| lisitsyn | raw pointers? why? :) | 17:50 |
| @wiking | do we and can we ever switch to some in swig interface? | 17:51 |
| @HeikoS | thought that was the plan? | 17:51 |
| lisitsyn | sure | 17:51 |
| @wiking | as soon as we drop the ctors that use args | 17:51 |
| @wiking | right? | 17:51 |
| lisitsyn | yes | 17:51 |
| @wiking | nmm | 17:52 |
| @wiking | there goes kwargs | 17:52 |
| @wiking | :) | 17:52 |
| @HeikoS | only cpp | 17:53 |
| @wiking | btw HeikoS | 17:53 |
| @wiking | shouldn't it go | 17:53 |
| @wiking | Some<const BinaryLabels> binary_labels(Some<const Labels*>) | 17:53 |
| @wiking | ? | 17:53 |
| @wiking | ? | 17:53 |
| @wiking | just for the sake of correctness? | 17:53 |
| @HeikoS | yes | 17:53 |
| @wiking | so that's already 4 | 17:53 |
| @wiking | right? | 17:53 |
| @wiking | meaning for each factory like this | 17:53 |
| @wiking | if u wanna do const then you wanna do 4 of them | 17:54 |
| @wiking | :) | 17:54 |
| @HeikoS | yes | 17:54 |
| @wiking | :> | 17:54 |
| @wiking | this is shity :) | 17:54 |
| @wiking | just saying | 17:54 |
| @wiking | :> | 17:54 |
| @wiking | no personal whatever just sayign that again | 17:54 |
| @wiking | this is way too much of blabla code | 17:54 |
| @HeikoS | it was the reason why the flag came up | 17:54 |
| @HeikoS | because of all those methods | 17:55 |
| @wiking | ok so one question then | 17:55 |
| @wiking | say that we coudl autofix it | 17:55 |
| -!- shubham808 [dfbda541@gateway/web/freenode/ip.223.189.165.65] has quit [Ping timeout: 260 seconds] | 17:55 | |
| @wiking | but having everywhere SGO* wrapped with Some | 17:55 |
| @wiking | should be doable? | 17:55 |
| @wiking | i mean say that somebody magically does it | 17:55 |
| @wiking | and we dont pass ever again | 17:55 |
| @wiking | SGO* | 17:55 |
| @wiking | anywhere | 17:56 |
| @HeikoS | I think there might be more issues | 17:56 |
| @HeikoS | subclassing | 17:56 |
| @wiking | like? | 17:56 |
| @wiking | could you elaborate? | 17:56 |
| @HeikoS | Some<const BinaryLabels> binary_labels(Some<const Labels*>) | 17:56 |
| @HeikoS | you cannot pass binary labels to this thing | 17:56 |
| @HeikoS | you have to cast it | 17:56 |
| @HeikoS | and this problem will appear in more places | 17:57 |
| @HeikoS | (i think) | 17:57 |
| @HeikoS | otherwise (re your question), i dont know | 17:57 |
| @HeikoS | hard to tell | 17:57 |
| @HeikoS | but this one I see | 17:57 |
| @HeikoS | (and encountered when playing around with this) | 17:57 |
| @wiking | mmm | 17:58 |
| @wiking | i mean first of all | 17:58 |
| @wiking | why would you pass Some<BinaryLabels> every to binary_labels? | 17:58 |
| @HeikoS | just an example | 17:58 |
| @wiking | but no | 17:58 |
| @HeikoS | maybe it is MulticlassLabels | 17:58 |
| @wiking | why would you do it? :) | 17:58 |
| @HeikoS | you wouldnt | 17:58 |
| @wiking | but you never pass | 17:59 |
| @wiking | MulticlassLabels | 17:59 |
| @HeikoS | in this case, you are right, you dont | 17:59 |
| @wiking | as MulticlassLabels to binary_labels | 17:59 |
| @wiking | right? | 17:59 |
| @HeikoS | because all you have is the base class pointer | 17:59 |
| @wiking | you always have CLables | 17:59 |
| @HeikoS | in CMachine | 17:59 |
| @HeikoS | yes | 17:59 |
| @HeikoS | for this one | 17:59 |
| @wiking | so | 17:59 |
| @HeikoS | that is not an issue | 17:59 |
| @wiking | ok so say that this is then not an issue | 17:59 |
| @HeikoS | it might be somewhere else though | 17:59 |
| @wiking | as we really just work with base calsses | 17:59 |
| @HeikoS | ok sure | 17:59 |
| @wiking | i mean if you use base classes | 17:59 |
| @wiking | wrapping some/shared_ptr | 18:00 |
| @wiking | shouldn't be an issue | 18:00 |
| @HeikoS | yep | 18:00 |
| @wiking | i'm just wondering that say we drop | 18:00 |
| @wiking | Some | 18:00 |
| @wiking | i mean drop * | 18:00 |
| @wiking | and use only Some | 18:00 |
| @wiking | would that mean that we only have to deal with Some<T> and Some<const T> | 18:00 |
| @HeikoS | think so | 18:01 |
| @wiking | i.e. we are back to 2 | 18:01 |
| @wiking | i mean if we want const correctness | 18:02 |
| @wiking | i guess this is the price you have to pay | 18:02 |
| @HeikoS | yes it is | 18:02 |
| @HeikoS | question is: is it worth to pay it? :D | 18:02 |
| @wiking | i mean if it's copy-paste then macro it | 18:02 |
| @HeikoS | macro is the third option, yes | 18:03 |
| @wiking | and that's it | 18:03 |
| @HeikoS | with this, I am a bit afraid that this will have to be done for a lot of code | 18:03 |
| @wiking | the question is whether we need or want | 18:03 |
| @wiking | const correctness | 18:03 |
| @wiking | :) | 18:03 |
| @HeikoS | for objects, yes | 18:03 |
| @HeikoS | for SG* we want it definitely | 18:04 |
| @wiking | if yes | 18:04 |
| @HeikoS | we have seen that this makes a big difference | 18:04 |
| @wiking | then that's it | 18:04 |
| @HeikoS | for objects, idk | 18:04 |
| @wiking | i mean Some only makes sense for SGO | 18:04 |
| @HeikoS | yeah sure | 18:05 |
| @HeikoS | I mean | 18:05 |
| @wiking | i.e. | 18:05 |
| @wiking | template <class T> | 18:05 |
| @wiking | class Some | 18:05 |
| @wiking | should have a type-trait | 18:05 |
| @HeikoS | the question whether we want const correctness only concerns Some | 18:05 |
| @wiking | for T being SGO derived :) | 18:05 |
| @HeikoS | one big argument for the const correcness that I have seen in shogun is the thread safety | 18:06 |
| @HeikoS | lisitsyn what is your opinion on doing macros for this? | 18:08 |
| -!- travis-ci [~travis-ci@ec2-54-160-138-195.compute-1.amazonaws.com] has joined #shogun | 18:08 | |
| travis-ci | it's Shubham Shukla's turn to pay the next round of drinks for the massacre he caused in shogun-toolbox/shogun: https://travis-ci.org/shogun-toolbox/shogun/builds/377778026 | 18:08 |
| -!- travis-ci [~travis-ci@ec2-54-160-138-195.compute-1.amazonaws.com] has left #shogun [] | 18:08 | |
| lisitsyn | HeikoS: I don't know | 18:10 |
| lisitsyn | HeikoS: I see no problem in binary_labels(...) in particular | 18:10 |
| lisitsyn | I mean if that's the only case | 18:11 |
| lisitsyn | of const T | 18:11 |
| lisitsyn | should be fine | 18:11 |
| @HeikoS | the ones I see are features and labels | 18:11 |
| lisitsyn | yeah if it is just copy-paste thing to be done once | 18:11 |
| lisitsyn | it is fine | 18:11 |
| @HeikoS | it gets more tricky if we have methods that accept a label and a feature | 18:11 |
| lisitsyn | yeah | 18:11 |
| lisitsyn | that's what I am afraid of | 18:12 |
| @HeikoS | or more | 18:12 |
| @HeikoS | wiking, lisitsyn then what about this approach: we copy/paste/macro it for now, and if there is a situation where the combinations explode, we can discuss again? | 18:12 |
| @wiking | probably | 18:12 |
| lisitsyn | I am fine | 18:12 |
| @wiking | btw | 18:12 |
| @wiking | when you have the new interface | 18:13 |
| @wiking | fit(X, y) | 18:13 |
| @wiking | what do you pass there? :) | 18:13 |
| @wiking | (const X, const y) ? | 18:13 |
| @HeikoS | haha | 18:13 |
| @HeikoS | there is the example | 18:13 |
| @wiking | right? | 18:13 |
| @HeikoS | yes I think both should be const | 18:13 |
| @wiking | i mean i would say const | 18:13 |
| @HeikoS | eventially | 18:13 |
| lisitsyn | I wouldn't bring much of constness into swig | 18:13 |
| @HeikoS | yes definitely | 18:13 |
| @wiking | ok i mean | 18:13 |
| @wiking | Some<> | 18:13 |
| lisitsyn | no target language has notion of const | 18:13 |
| @wiking | indeed | 18:14 |
| @wiking | but passing | 18:14 |
| lisitsyn | so bringing it to user interface is bothersome | 18:14 |
| @wiking | non-const * | 18:14 |
| @wiking | to const * | 18:14 |
| @wiking | is ok | 18:14 |
| @wiking | :) | 18:14 |
| lisitsyn | for swig Some<Features> and Some<const Features> are completely different | 18:14 |
| lisitsyn | hence you would need to cast it all the time | 18:14 |
| lisitsyn | that's why I think we shouldn't bring consts here | 18:14 |
| @wiking | mmm | 18:15 |
| @HeikoS | i think this can be solved with an interface method | 18:15 |
| @wiking | what i meant htat in c++ side you want to have those const | 18:15 |
| @wiking | in the fit/train method | 18:15 |
| @wiking | now how do you interface this to the swig | 18:15 |
| @wiking | that's a different story | 18:15 |
| lisitsyn | yeah and that's why I suggested to lock things runtime | 18:15 |
| lisitsyn | :D | 18:15 |
| @wiking | :> | 18:15 |
| @wiking | mmm | 18:16 |
| @wiking | we should then have SWIG specific | 18:16 |
| @wiking | Some | 18:16 |
| @wiking | or something | 18:16 |
| @HeikoS | I think that is ok | 18:16 |
| lisitsyn | Some is already swig-specific, no? :) | 18:16 |
| @wiking | no | 18:16 |
| @wiking | HeikoS, is using some now | 18:16 |
| @wiking | for c++ only | 18:16 |
| @wiking | :D | 18:16 |
| @HeikoS | there is another SWIG interface that accepts the non-const, and then is calls the const version | 18:16 |
| @HeikoS | lisitsyn yes, I am | 18:16 |
| @HeikoS | this is an internal helper method | 18:16 |
| @wiking | i mean i get you rpoint in the runtime thing but that should only happen | 18:17 |
| @wiking | maxiumum | 18:17 |
| @wiking | on swig side | 18:17 |
| @HeikoS | what sucks about the additional SWIg method is that we will have to do it everytime that a c++ base interface accepts const | 18:17 |
| @wiking | or the swig wrapper | 18:17 |
| lisitsyn | yes sure | 18:17 |
| lisitsyn | I am talking about swig | 18:17 |
| lisitsyn | once it gets to swig we have troubles | 18:17 |
| @wiking | but in the c++ side you wanna keep these things | 18:18 |
| @wiking | i.e. do const checking compile time | 18:18 |
| @wiking | ;) | 18:18 |
| @HeikoS | lisitsyn what is the data strcuture for internal pointer smart managing then? | 18:18 |
| lisitsyn | HeikoS: depends | 18:18 |
| lisitsyn | you can do raw pointers | 18:18 |
| lisitsyn | if it is to be scoped inside the function | 18:19 |
| lisitsyn | it is fine to do raw ptrs | 18:19 |
| @HeikoS | ok | 18:19 |
| @HeikoS | so then | 18:19 |
| @wiking | SomeSwig<> | 18:19 |
| @wiking | :) | 18:19 |
| @HeikoS | mmh lisitsyn but there is a lot of other situations | 18:20 |
| @HeikoS | like the members | 18:20 |
| @HeikoS | that we SG_UNREF in the dtor | 18:20 |
| @HeikoS | why dont we use shared_ptr internally then? | 18:20 |
| @HeikoS | or whatever | 18:20 |
| @wiking | because then | 18:20 |
| @wiking | how do you mix match | 18:20 |
| @HeikoS | yes | 18:20 |
| @wiking | shared_ptr and some | 18:20 |
| @wiking | :) | 18:20 |
| @HeikoS | and how to mix and match SomeSwig and Some? | 18:21 |
| @HeikoS | OR | 18:21 |
| @wiking | it's fine | 18:21 |
| @wiking | they use the same ref-counter | 18:21 |
| @wiking | :) | 18:21 |
| @HeikoS | mmh | 18:21 |
| @wiking | shared_ptr and some has totally different ref coutner | 18:21 |
| @HeikoS | yeah I see | 18:21 |
| @wiking | shared_ptr has it's own ref counter | 18:21 |
| @wiking | some is acting on SGO::RefCounter | 18:21 |
| @HeikoS | ok then we can have a SwigSome that has the readlonly flag | 18:21 |
| @HeikoS | and the Some we have | 18:21 |
| @HeikoS | where we suck up the double code | 18:22 |
| @wiking | i mean SwigSome | 18:22 |
| @wiking | is just a glueing stuff | 18:22 |
| @wiking | for swig iface | 18:22 |
| @HeikoS | yes | 18:22 |
| @HeikoS | and then eventually | 18:22 |
| @wiking | it shouldn't appear anywhere in c++ code itself | 18:22 |
| @HeikoS | methods like | 18:22 |
| @HeikoS | CMachine::train | 18:22 |
| @HeikoS | we will expose a special SwigSome version of it to Swiug | 18:22 |
| @wiking | yeah but jut define it in .i | 18:22 |
| @HeikoS | and this one will call CMachine::train(Some<const CFeatures>) | 18:23 |
| @HeikoS | yes | 18:23 |
| @HeikoS | exactly | 18:23 |
| @HeikoS | like for put | 18:23 |
| @HeikoS | so this way | 18:23 |
| @HeikoS | we have a consistent looking interface | 18:23 |
| @wiking | you dont wanna contaminate the c++ code with that shit | 18:23 |
| @HeikoS | but the c++ one is a bit smarter and still type safe | 18:23 |
| @HeikoS | not so sure whats better there | 18:23 |
| @HeikoS | this is a lot of code | 18:23 |
| @wiking | you can add c++ code | 18:23 |
| @wiking | to the swig interface itself | 18:23 |
| @HeikoS | and hacking c++ code that is generated from swig is not really a good workflow | 18:23 |
| @wiking | see sg_print ...cpp | 18:24 |
| @HeikoS | ah yeah | 18:24 |
| @wiking | you dont have to write the whole thing within .i | 18:24 |
| @HeikoS | well, still takes ages | 18:24 |
| @wiking | what? | 18:24 |
| @HeikoS | compiling ...but this will be better soon anyways | 18:24 |
| @wiking | i mean in that case | 18:24 |
| @wiking | you dont have to compile | 18:24 |
| @wiking | the swig iface | 18:24 |
| @wiking | just that particular .cpp | 18:24 |
| @wiking | :) | 18:24 |
| @wiking | you can even write tests over that | 18:25 |
| @HeikoS | ok cool | 18:25 |
| @HeikoS | yeah I see | 18:25 |
| @wiking | so that you dont require | 18:25 |
| @wiking | compiling the whole swig iface | 18:25 |
| @HeikoS | get it | 18:25 |
| @wiking | just write a simple unit test | 18:25 |
| @HeikoS | lisitsyn, so we use Some internally as well | 18:25 |
| @wiking | that uses those wrappers | 18:25 |
| @HeikoS | e.g. as a member field? | 18:25 |
| @wiking | HeikoS, already happenbing some placess | 18:25 |
| @HeikoS | wiking: you have an example? | 18:25 |
| @wiking | i mean Unique | 18:25 |
| lisitsyn | I don't see any point of using two different pointer holders | 18:26 |
| @HeikoS | member of SGObject | 18:26 |
| lisitsyn | yeah Unique is one thing | 18:26 |
| @HeikoS | lisitsyn: explain pls | 18:26 |
| @wiking | lisitsyn, you can derive it from Some :) | 18:26 |
| lisitsyn | I am a bit lost | 18:26 |
| @wiking | HeikoS, SGObject::self | 18:26 |
| lisitsyn | what's the problem we're solving? | 18:26 |
| lisitsyn | :) | 18:26 |
| @wiking | but that's not some | 18:26 |
| @wiking | but unique | 18:26 |
| @wiking | but that's not part of the members | 18:26 |
| @wiking | as that's the members holder itself | 18:27 |
| @wiking | lisitsyn, i mean say we go with the runtime | 18:27 |
| @HeikoS | lisitsyn: the problem of say CMachine::train | 18:27 |
| @HeikoS | and the type of its inputs | 18:27 |
| @HeikoS | should be Some<const ...> for c++ | 18:27 |
| @HeikoS | but Some<...> for swig | 18:27 |
| lisitsyn | HeikoS: yeah but our target is not C++ | 18:27 |
| @wiking | lisitsyn, not only :) | 18:27 |
| lisitsyn | yeah or rather that | 18:28 |
| @wiking | or you wanna expose Some only on swig | 18:28 |
| @wiking | meaning that keep the train(raw ptr) sotry? | 18:28 |
| @wiking | in c++ | 18:28 |
| @wiking | i mean we wanna get rid of the shitty | 18:28 |
| @wiking | SG_(UN)REF stuff | 18:28 |
| @wiking | in c++ | 18:28 |
| @wiking | i.e. -> Some | 18:28 |
| -!- travis-ci [~travis-ci@ec2-184-73-70-8.compute-1.amazonaws.com] has joined #shogun | 18:28 | |
| travis-ci | it's Shubham Shukla's turn to pay the next round of drinks for the massacre he caused in shogun-toolbox/shogun: https://travis-ci.org/shogun-toolbox/shogun/builds/377778026 | 18:28 |
| -!- travis-ci [~travis-ci@ec2-184-73-70-8.compute-1.amazonaws.com] has left #shogun [] | 18:28 | |
| @wiking | so if we convert all our raw ptrs to Some | 18:29 |
| @HeikoS | there is two problems. 1. is ^.... and 2 is the one with CMachine::train | 18:29 |
| lisitsyn | yeah that's why I think runtime consts is good :D | 18:29 |
| @wiking | now the problem is that constness in swig | 18:29 |
| @HeikoS | it would solve both | 18:29 |
| @wiking | is foobar | 18:29 |
| @HeikoS | with a single Some | 18:29 |
| @HeikoS | and no macros | 18:30 |
| @wiking | yeah but you loose | 18:30 |
| @wiking | the whoel compiler feature | 18:30 |
| @HeikoS | and no additional swig code | 18:30 |
| @wiking | right? :) | 18:30 |
| @HeikoS | yes | 18:30 |
| @HeikoS | :) | 18:30 |
| @wiking | i mean i really dont understand then | 18:30 |
| @wiking | why do we use a compiler? | 18:30 |
| @wiking | we should just write a code in an interpreted language | 18:30 |
| @wiking | :) | 18:30 |
| @wiking | it is a great thing | 18:30 |
| @HeikoS | sure | 18:30 |
| @wiking | to have the compiler figure out things for you | 18:31 |
| lisitsyn | not really | 18:31 |
| @wiking | and you dont need to do those things in runtime | 18:31 |
| lisitsyn | I mean we have to live with that we target to some languages | 18:31 |
| @wiking | imo | 18:31 |
| lisitsyn | that don't have consts | 18:31 |
| @wiking | yeah | 18:31 |
| lisitsyn | but in C++ we will still use consts | 18:31 |
| @wiking | but why does the c++ has to suffer that part? | 18:31 |
| @wiking | i mean i have a feeling | 18:31 |
| @wiking | that currently everything is being sacrificed over interpretable languages desk | 18:31 |
| @wiking | and we push everything to be runtime | 18:32 |
| @HeikoS | lisitsyn: so I actually think the target lang not having const is easy to fix | 18:32 |
| @HeikoS | via gluing code, so no casting needed | 18:32 |
| @wiking | i mean swig allows us to do some tricks | 18:32 |
| @wiking | when gluing shit together | 18:32 |
| lisitsyn | I don't know | 18:32 |
| @HeikoS | i have another q | 18:33 |
| @HeikoS | foo(const CLabels*) | 18:33 |
| lisitsyn | it is not clear yet what's best | 18:33 |
| @HeikoS | I can call that with CLabels* or with const CLabels* | 18:33 |
| @wiking | HeikoS, yep | 18:33 |
| @HeikoS | but foo(Some<const CLabels*>) | 18:33 |
| @HeikoS | I cannot call with Some<CLabels*> | 18:34 |
| lisitsyn | oh that's easy to implement | 18:34 |
| lisitsyn | I think this implicit cast can be implemented | 18:34 |
| @HeikoS | if this works, then I dont even need to do macros | 18:34 |
| @HeikoS | as these dispatcher methods for CMachine stuff will never alter the data | 18:35 |
| lisitsyn | just patch it then :P | 18:35 |
| lisitsyn | let me show you where | 18:35 |
| lisitsyn | ah wait | 18:35 |
| lisitsyn | you want Some<CLabels> -> Some<const CLabels>? | 18:35 |
| lisitsyn | ok so adding const, right? | 18:35 |
| @HeikoS | yes | 18:36 |
| lisitsyn | https://github.com/shogun-toolbox/shogun/blob/develop/src/shogun/base/some.h#L20 | 18:36 |
| lisitsyn | HeikoS: ^ add one more here | 18:36 |
| @HeikoS | so I can offer methods that just read from labels | 18:36 |
| lisitsyn | std::remove_const | 18:36 |
| lisitsyn | I mean ctor that takes Some<T without a const> | 18:36 |
| lisitsyn | strange shared_ptr doesn't have this one | 18:37 |
| lisitsyn | it seems rather safe to me | 18:37 |
| @HeikoS | can you be a bit more explicit? | 18:39 |
| lisitsyn | there is Some(const Some<T>& other); | 18:39 |
| lisitsyn | add Some(const Some<std::remove_const<T>>& other); | 18:39 |
| @wiking | btw | 18:40 |
| lisitsyn | good? :) | 18:40 |
| @wiking | when you pass const T* | 18:40 |
| @HeikoS | lisitsyn: thx | 18:41 |
| @wiking | to a function | 18:41 |
| @wiking | when do you wanna hold a ref to it after finishing the function itself? | 18:41 |
| lisitsyn | yolo? | 18:42 |
| lisitsyn | don't get it | 18:42 |
| @wiking | train | 18:42 |
| @wiking | for example | 18:42 |
| lisitsyn | ah | 18:42 |
| @wiking | do you really fucking need to Some? | 18:42 |
| lisitsyn | yes because this makes things safe | 18:42 |
| @wiking | cound't you just actually pass const Features^ | 18:42 |
| @wiking | & | 18:42 |
| lisitsyn | raw ptr is only fine if you don't own things | 18:43 |
| @wiking | when do you pass things that you own and they are const? | 18:43 |
| @wiking | or you suppose to own but its actually const | 18:43 |
| lisitsyn | no, because it won't play good with swig | 18:43 |
| lisitsyn | I don't see references and swig play nice together | 18:44 |
| @wiking | yeah but that's a gluing problem | 18:44 |
| @wiking | i'm really just talking now about c++ interface | 18:44 |
| @wiking | for the sake of argument | 18:44 |
| lisitsyn | even for C++ I think pointers are much more flexible | 18:44 |
| @wiking | that if you pass a const T* | 18:45 |
| @wiking | then you really dont need to deal with the ref count | 18:45 |
| @wiking | or shouldn't | 18:45 |
| @wiking | unless you pass the ownership | 18:45 |
| @wiking | but if you pass ownership then... | 18:45 |
| @wiking | ? | 18:45 |
| lisitsyn | then you need refcount ;) | 18:45 |
| lisitsyn | oh btw | 18:46 |
| lisitsyn | this is a good note | 18:46 |
| lisitsyn | how Some<const T> is going to work? | 18:46 |
| lisitsyn | :D | 18:46 |
| @wiking | :D | 18:46 |
| lisitsyn | it should call ref() | 18:46 |
| lisitsyn | which is obviously non-const | 18:46 |
| lisitsyn | HeikoS: how did it work? | 18:46 |
| @wiking | mutable | 18:46 |
| @wiking | :D | 18:46 |
| @HeikoS | lisitsyn: conflicts | 18:46 |
| @HeikoS | Some(const Some<std::remove_const<T>>& other); | 18:47 |
| lisitsyn | HeikoS: ref is not const | 18:47 |
| @wiking | lisitsyn, mutuable RefCount* wouldn't work? | 18:47 |
| @HeikoS | lisitsyn: so? | 18:47 |
| lisitsyn | but that's like playing with a shotgun | 18:47 |
| lisitsyn | :D | 18:47 |
| @wiking | yes | 18:47 |
| @wiking | indeed | 18:47 |
| @wiking | :) | 18:47 |
| lisitsyn | HeikoS: how ref() is going to work? | 18:47 |
| lisitsyn | in the some | 18:47 |
| lisitsyn | Some<const T> should not even work | 18:48 |
| lisitsyn | did it compile? | 18:48 |
| @wiking | i mean this is why better that the ref count is actually out of the SGO :P and is in the wrapper (say Some) | 18:48 |
| lisitsyn | yeah but it is here | 18:48 |
| @wiking | yeye | 18:48 |
| @wiking | i mnow | 18:48 |
| @wiking | *know | 18:48 |
| @wiking | just saying that currently yeah it's foobar | 18:48 |
| lisitsyn | HeikoS: Some<const T> should not compile | 18:48 |
| lisitsyn | :D | 18:48 |
| lisitsyn | if it did I don't understand how could it | 18:49 |
| lisitsyn | oh sorry guys have to go | 18:49 |
| @wiking | lisitsyn, ttyl | 18:49 |
| @wiking | but i thunk | 18:49 |
| @wiking | atm really Some<const T> | 18:49 |
| @wiking | is really an oximoron | 18:49 |
| @wiking | in a way | 18:49 |
| @wiking | because Some allows you shared ownership | 18:50 |
| @wiking | where const actually somehow pushing you or telling you that you actually down own this thing | 18:50 |
| @wiking | HeikoS, http://coliru.stacked-crooked.com/a/c2357e9e83f886c7 | 18:52 |
| @HeikoS | sorry I wasnt paying attention for a while what is this? | 18:53 |
| @wiking | simple example how in case of shared_ptr<const T> works automagically | 18:54 |
| @wiking | but yeah lisitsyn had a good point | 18:54 |
| @wiking | with teh current design | 18:54 |
| @wiking | Some<const SGObject> | 18:54 |
| @wiking | shouldn't even compile | 18:54 |
| @wiking | as it should call ref() on the passed SGO | 18:55 |
| @wiking | that is modifying a member element of SGO | 18:55 |
| @wiking | which shoulnd't be possible | 18:55 |
| @wiking | because it's const | 18:55 |
| @HeikoS | ah yeah so your example shows it works | 18:55 |
| @wiking | yeah with shared_ptr | 18:55 |
| @HeikoS | and yes I guess ref is non const so it cannot compile | 18:55 |
| @wiking | but that's a very different story | 18:55 |
| @wiking | as the ref coutner is withing | 18:55 |
| @HeikoS | uh | 18:55 |
| @wiking | *within shared_ptr | 18:55 |
| @HeikoS | so are we back to square one then? :D | 18:55 |
| @wiking | well | 18:56 |
| @wiking | moving out the ref counter | 18:56 |
| @wiking | to some | 18:56 |
| @wiking | would solve the problem | 18:56 |
| @wiking | :DDD | 18:56 |
| @wiking | but that's gonna be a huge patch btw | 18:56 |
| @wiking | as you need to remove the whole sg_ref story | 18:56 |
| @wiking | etc etc | 18:56 |
| @HeikoS | what do you mean with moving out the ref counter? | 18:56 |
| @wiking | meaning that Some is actually becoming more and more like shared_ptr | 18:57 |
| @wiking | i.e. that the ref counter is in Some | 18:57 |
| @wiking | not in SGO | 18:57 |
| @wiking | which of course makes a lot of sense | 18:57 |
| @wiking | :P | 18:57 |
| @wiking | because if you want const SGO | 18:57 |
| @wiking | but see my comment | 18:57 |
| @wiking | [18:49] <wiking> because Some allows you shared ownership | 18:58 |
| @wiking | [18:50] <wiking> where const actually somehow pushing you or telling you that you actually down own this thing | 18:58 |
| @HeikoS | yes | 18:58 |
| @HeikoS | mmh | 18:58 |
| @wiking | that's why this concept of Some<const T> is contradictory in a way | 18:58 |
| @HeikoS | foobar | 18:58 |
| @HeikoS | indeed | 18:58 |
| @HeikoS | gna | 18:58 |
| @wiking | and you actually just wanna pass | 18:58 |
| @wiking | const T* or const T^ | 18:58 |
| @wiking | const T* or const T^ | 18:58 |
| @wiking | T& | 18:58 |
| @wiking | i mean T const* | 18:58 |
| @wiking | or const T& | 18:58 |
| @HeikoS | I think I will change this dispatcher methods back to raw pointers for now | 18:59 |
| @wiking | and you sholuld be passing some<T> if you wanna share ownership | 18:59 |
| @wiking | but now i have to go for 20 mins | 18:59 |
| @wiking | ttyl | 18:59 |
| @wiking | i mean bbl | 18:59 |
| @HeikoS | ok ill be gone by then probably | 18:59 |
| -!- iglesiasg [~iglesias@f119189.upc-f.chello.nl] has joined #shogun | 18:59 | |
| @HeikoS | see you | 18:59 |
| @wiking | iglesiasg, hola | 18:59 |
| @HeikoS | iglesiasg: jo! | 18:59 |
| @HeikoS | missed some good discussions :D | 18:59 |
| @wiking | HeikoS, btw | 18:59 |
| @wiking | i have looked into tf's random | 18:59 |
| iglesiasg | from today? | 18:59 |
| @HeikoS | iglesiasg: just literally ended | 19:00 |
| @wiking | those hax0rs | 19:00 |
| @wiking | solve the story actually writing their own | 19:00 |
| iglesiasg | will go to logs :) | 19:00 |
| @wiking | NormalDistrib | 19:00 |
| @wiking | and UniformDistrib | 19:00 |
| @wiking | (in order to circumvent the differences | 19:00 |
| @wiking | between libc++ and libstdc++ | 19:00 |
| @HeikoS | interesting | 19:00 |
| @HeikoS | just like us :) | 19:00 |
| @wiking | they use the prng of c++11 | 19:00 |
| @wiking | since those are the same | 19:00 |
| @wiking | but everything else | 19:01 |
| @wiking | ah and one more thing :) | 19:01 |
| @HeikoS | and then transform themselves? | 19:01 |
| @wiking | yes | 19:01 |
| @wiking | those hax0rs | 19:01 |
| @wiking | actually have 1 PRNG | 19:01 |
| @wiking | for the seed | 19:01 |
| @wiking | :P | 19:01 |
| @wiking | which is global | 19:01 |
| @wiking | :D | 19:01 |
| @HeikoS | lol | 19:01 |
| @HeikoS | lol | 19:01 |
| @wiking | so if you run multiple sessions of TF | 19:01 |
| @wiking | you are acutally | 19:01 |
| @wiking | lalalla | 19:01 |
| @wiking | crazy man | 19:01 |
| @wiking | :) | 19:01 |
| @wiking | i thought we are shitty | 19:02 |
| @wiking | :DDD | 19:02 |
| @HeikoS | we are | 19:02 |
| @HeikoS | :D | 19:02 |
| @wiking | but now i realise that actually we are doing industry standard | 19:02 |
| @wiking | :D | 19:02 |
| @HeikoS | shogun is production code | 19:02 |
| @wiking | yeah i mean | 19:02 |
| @HeikoS | qualityzzzzz | 19:02 |
| @wiking | honestly | 19:02 |
| @wiking | wehn i saw this one | 19:02 |
| @wiking | i'm like | 19:02 |
| @wiking | "facepalm: this is what we did like 6 years ago" | 19:02 |
| @wiking | :> | 19:02 |
| @wiking | anyhow | 19:02 |
| @wiking | now i really gotta go | 19:03 |
| @wiking | be back in 20 | 19:03 |
| @wiking | bbl | 19:03 |
| @HeikoS | see you | 19:03 |
| @wiking | https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/lib/random/random.cc#L36 | 19:03 |
| @wiking | \o/ | 19:03 |
| @wiking | note the static | 19:03 |
| @wiking | ! | 19:03 |
| @wiking | and the fucking mutex | 19:03 |
| @wiking | :D | 19:03 |
| @wiking | https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/lib/random/random_distributions.h#L407 | 19:04 |
| @wiking | https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/lib/random/random_distributions.h#L646 | 19:05 |
| @wiking | :) | 19:05 |
| @wiking | lalala | 19:05 |
| iglesiasg | HAHAHA | 19:05 |
| @wiking | we already have this in shogun | 19:05 |
| @wiking | \o/ | 19:05 |
| iglesiasg | oh the capitals | 19:05 |
| iglesiasg | appropriate still though | 19:05 |
| @wiking | and actually i think our uniform dist -> norm dist actually i sfaster | 19:05 |
| @wiking | :P | 19:05 |
| iglesiasg | went quickly through the logs - why Some<const T> is non-sense? | 19:05 |
| @wiking | (never gonna leave the office) | 19:06 |
| iglesiasg | :) | 19:06 |
| iglesiasg | hahaha | 19:06 |
| iglesiasg | we can take it later too | 19:06 |
| @wiking | because that says that hey | 19:06 |
| @wiking | i want to read-only the stuff | 19:06 |
| @wiking | for the time i'm running this function | 19:06 |
| @wiking | and wanna have ownership over it | 19:06 |
| @wiking | after the function has finished | 19:06 |
| @wiking | right? | 19:06 |
| iglesiasg | if the pointer is local to the function | 19:07 |
| @HeikoS | wiking: there is a problem with the dispatcher | 19:07 |
| @wiking | so actually you wanna own something that you can only read | 19:07 |
| @HeikoS | wiking: so it takes a const CLabels* | 19:07 |
| @HeikoS | what does it return? | 19:07 |
| @wiking | HeikoS, only const ) | 19:07 |
| @HeikoS | say it actually has to convert things | 19:07 |
| iglesiasg | then after the function finishes it will have gone out of scope so then its "time of ownership" finishes as well | 19:07 |
| @HeikoS | so it needs to create a new instance | 19:07 |
| @wiking | iglesiasg, yeah but then whhy do you have to Some? :) | 19:07 |
| @wiking | i mean if you only pass the reference to it and use it only in function | 19:08 |
| iglesiasg | wiking, because you want to make sure that the underlying memory is not freed | 19:08 |
| iglesiasg | wiking, you want "read-only ownership" | 19:08 |
| iglesiasg | wiking, that makes sense I think, no? | 19:08 |
| @wiking | iglesiasg, what's the scenario | 19:08 |
| @wiking | when you actually would be able to do this | 19:08 |
| @wiking | Features* f; | 19:09 |
| iglesiasg | wiking, a performance evaluator for instance | 19:09 |
| @wiking | machine->train(f); | 19:09 |
| iglesiasg | wiking, you get a couple of sets of labels to compare against each other | 19:09 |
| iglesiasg | wiking, so you want to read them but not modify them | 19:09 |
| @wiking | and then i do a delete f | 19:09 |
| @wiking | i mean this only would fuck you up | 19:09 |
| iglesiasg | why? | 19:09 |
| @wiking | if say you do the training in a back-thread | 19:09 |
| iglesiasg | I don | 19:09 |
| iglesiasg | oeps | 19:09 |
| @wiking | and do the delete before the thread finishes | 19:10 |
| @wiking | or? | 19:10 |
| iglesiasg | but why delete? | 19:10 |
| @wiking | i mean how else i'm gonna kill the memory | 19:10 |
| @wiking | before the train finishes with f | 19:10 |
| @wiking | :) | 19:10 |
| iglesiasg | when the shared ptr goes of scope it decreases the ref count, no? | 19:10 |
| @wiking | but say that we dont have now | 19:10 |
| @wiking | shared_ptr or some | 19:10 |
| iglesiasg | and frees memory only if none else is holding that | 19:10 |
| @wiking | i mean i'm just trying to see | 19:10 |
| @wiking | how is it that you pass a const poitner/reference | 19:11 |
| @wiking | and suddenly while running that function | 19:11 |
| @wiking | the memory would disappear | 19:11 |
| iglesiasg | ah ok, I think I understand your concern | 19:11 |
| @wiking | i mean i can now only think of a scenario | 19:11 |
| @wiking | where you actually doing a bad coding practice | 19:11 |
| iglesiasg | I think a better way of thinking about this problem is in the way why smart pointers are really useful | 19:12 |
| @wiking | yeah but they are useful | 19:12 |
| @wiking | for shared ownership | 19:12 |
| @wiking | but then you actually do | 19:12 |
| @wiking | shared_ptr<T> | 19:12 |
| @wiking | not a const | 19:12 |
| iglesiasg | and it is when the pattern malloc .... do stuff ... free is not that clear | 19:12 |
| @wiking | because you actually wanna do some hacks with T | 19:12 |
| @wiking | most probably change it | 19:12 |
| @wiking | some way or another | 19:12 |
| iglesiasg | mmm I see | 19:13 |
| @wiking | if you just read it | 19:13 |
| @wiking | then i dont see why would you need to wrap it | 19:13 |
| @wiking | with a shared_ptr | 19:13 |
| iglesiasg | I still read think shared_ptr<const T> is fair enough | 19:13 |
| @wiking | iglesiasg, yeah i. mean i'm not saying tha tyou cannot compile a code like that | 19:13 |
| @HeikoS | iglesiasg: I have a riddle for you | 19:13 |
| @wiking | but thinking in Bjorn's way | 19:13 |
| iglesiasg | wiking, yeah | 19:13 |
| iglesiasg | wiking, I am saying that it is useful | 19:13 |
| @wiking | you always wanna pass things the most simple way you can | 19:13 |
| iglesiasg | mmmm no no | 19:13 |
| @wiking | iglesiasg, i'm saying that i dont see the use-case | 19:13 |
| iglesiasg | the simplest way would be all non-const | 19:14 |
| @wiking | where it actually makes sense | 19:14 |
| iglesiasg | because that's C++ default :) | 19:14 |
| iglesiasg | but we know is not good | 19:14 |
| @wiking | iglesiasg, i mena not even sure | 19:14 |
| @wiking | whether in a ctor of an obj | 19:14 |
| @wiking | would make sense to pass a shared_ptr<const T> | 19:15 |
| @wiking | why wouldn't you just const T^ | 19:15 |
| @wiking | & | 19:15 |
| @wiking | ? | 19:15 |
| iglesiasg | of course! | 19:15 |
| iglesiasg | that I understand | 19:15 |
| iglesiasg | and I think it is a very good question | 19:15 |
| @wiking | i mean currently imo | 19:16 |
| @wiking | the ref coutner story of SGO | 19:16 |
| iglesiasg | but then there is the case when you have data that you don't use with value semantics like T | 19:16 |
| @wiking | was actually introduced because of swig | 19:16 |
| iglesiasg | you always have a * | 19:16 |
| iglesiasg | e.g. SGObject | 19:16 |
| @wiking | to be able to have ref() unref() | 19:16 |
| @wiking | and define this | 19:16 |
| @wiking | %feature("ref") shogun::CSGObject "SG_REF($this);" | 19:16 |
| @wiking | %feature("unref") shogun::CSGObject "SG_UNREF($this);" | 19:16 |
| @HeikoS | the fact that the refcounter is in the object is so weir | 19:17 |
| @HeikoS | d | 19:17 |
| @HeikoS | iglesiasg: hey maybe you have an idea here | 19:17 |
| iglesiasg | HeikoS, is this same as the riddle? | 19:17 |
| iglesiasg | :D | 19:17 |
| @HeikoS | yes | 19:17 |
| @wiking | HeikoS, that's because of swig | 19:17 |
| @HeikoS | :D | 19:17 |
| @HeikoS | in CMachine::train | 19:17 |
| @HeikoS | we work with CMachine::labels | 19:17 |
| @HeikoS | right? | 19:18 |
| iglesiasg | ok | 19:18 |
| @HeikoS | and since we only have base pointer | 19:18 |
| @HeikoS | we need to dispatch it | 19:18 |
| iglesiasg | ok | 19:18 |
| @HeikoS | we used to just enforce the right type using dynamic_cast before | 19:18 |
| @HeikoS | but now we want some automatic thing | 19:21 |
| iglesiasg | it makes sense the dynamic thing yeah | 19:21 |
| iglesiasg | it is casting still of course but good | 19:21 |
| @HeikoS | but now it would be better to just say | 19:21 |
| iglesiasg | automatic in what sense? | 19:21 |
| @HeikoS | binay_labels(m_labels) | 19:21 |
| @HeikoS | CbinaryLabels* binay_labels(m_labels) | 19:21 |
| @HeikoS | if it is possible, it gives you the pointer | 19:21 |
| @HeikoS | if not, it moans | 19:21 |
| @HeikoS | it might convert internally, or just cast | 19:21 |
| iglesiasg | ok | 19:21 |
| iglesiasg | how is it automatically? | 19:21 |
| @HeikoS | say it might convert from [0,1] representation to [-1,+1] | 19:21 |
| iglesiasg | the subclass type is in the function name, no? | 19:21 |
| @HeikoS | if m_labels was multiclass labels for example | 19:21 |
| @HeikoS | CBinaryLabels* binay_labels(CLabels*) | 19:21 |
| @HeikoS | automatic in the sense that it checks what is in the argument and then either casts or converts | 19:21 |
| iglesiasg | oh ok | 19:21 |
| @HeikoS | iglesiasg: so far so good | 19:21 |
| @HeikoS | we have that actually | 19:21 |
| iglesiasg | so this about the multiclass labels to binary case? | 19:21 |
| @HeikoS | https://github.com/shogun-toolbox/shogun/blob/develop/src/shogun/labels/BinaryLabels.cpp#L161 | 19:21 |
| @HeikoS | and here is an example where it converts | 19:22 |
| @HeikoS | https://github.com/shogun-toolbox/shogun/blob/develop/src/shogun/labels/MulticlassLabels.cpp#L226 | 19:22 |
| @HeikoS | https://github.com/shogun-toolbox/shogun/blob/develop/src/shogun/labels/MulticlassLabels.cpp#L196 | 19:22 |
| iglesiasg | ok | 19:23 |
| @wiking | btw /me wonders why we dont have a public member of each machine describing its label type? :) say something like using label_type = CMulticlassLabel for MulticlassMachine | 19:23 |
| @HeikoS | iglesiasg: so now | 19:24 |
| @HeikoS | I dont modify the passed argument right? | 19:24 |
| iglesiasg | wiking, something like typeid? | 19:24 |
| @wiking | so like Machine::label_type would actually always have the right label type... | 19:24 |
| @wiking | so when you do m->apply | 19:24 |
| iglesiasg | wiking, oh not only like typeid, because it is about the label type from the machine | 19:24 |
| iglesiasg | HeikoS, yeah | 19:25 |
| @wiking | then you could simply do m->apply()->as<m::label_type>() :) | 19:25 |
| @wiking | so you get the right label based on the machine | 19:25 |
| @wiking | :) | 19:25 |
| @HeikoS | iglesiasg: ok so why not make the argument const | 19:25 |
| iglesiasg | HeikoS, what about as? | 19:25 |
| @wiking | that cast should always work | 19:25 |
| @HeikoS | iglesiasg: as just tries to cast | 19:26 |
| @HeikoS | but we want something more here | 19:26 |
| @wiking | meaning you dont even need to do dynamic cast btw | 19:26 |
| @HeikoS | we want to also potentially convert | 19:26 |
| @HeikoS | say I have DenseLabels with data | 19:26 |
| @HeikoS | I cannot cast that to BinaryLabels | 19:26 |
| @HeikoS | I need to create a BinaryLabels instance, and pass the SGVector of the label data | 19:26 |
| @HeikoS | wiking: interesting idea! | 19:27 |
| @wiking | anyhow just an idea, but i think we could write helpers around this using swig that maybe on the end of the day when you do apply you actually right way get the right casted | 19:27 |
| @wiking | label | 19:27 |
| @wiking | instead of having to do it manually | 19:27 |
| @wiking | with the factory methods | 19:27 |
| @wiking | ;P | 19:27 |
| @HeikoS | we actually dont need factories in swig | 19:27 |
| @HeikoS | just CLabels | 19:27 |
| @HeikoS | and then .get | 19:27 |
| @wiking | never? | 19:27 |
| @HeikoS | iglesiasg: you see what I mean? | 19:27 |
| @HeikoS | wiking: no never | 19:27 |
| iglesiasg | HeikoS, BinaryLabels are DenseLabels | 19:27 |
| @HeikoS | iglesiasg: true | 19:28 |
| @wiking | HeikoS, coz u push everything inside c++? | 19:28 |
| @wiking | yeah ok because we always do | 19:28 |
| @HeikoS | but can I cast DenseLabels to BinaryLabels? | 19:28 |
| @wiking | base classes | 19:28 |
| @wiking | gotcha | 19:28 |
| @HeikoS | wiking: yes we just have CLabels | 19:28 |
| @wiking | but on the other hand | 19:28 |
| iglesiasg | HeikoS, dynamic_cast no? | 19:28 |
| @HeikoS | it is the training data that requires dispatching | 19:28 |
| @HeikoS | iglesiasg: no | 19:28 |
| @wiking | this could save some dynamic_Casts here and there | 19:28 |
| @HeikoS | I cannot cast a base type instance to sub-type isntance | 19:28 |
| @wiking | of course some machines would need to be split in this case | 19:29 |
| @wiking | :P | 19:29 |
| @wiking | which support both classification and regression | 19:29 |
| @wiking | anyhow | 19:29 |
| @wiking | i really wanna go home | 19:29 |
| @wiking | bbl | 19:29 |
| iglesiasg | HeikoS, you mean in general that that is not possible? | 19:29 |
| @HeikoS | not that I know | 19:29 |
| @HeikoS | wiking: bye again :) | 19:29 |
| iglesiasg | mmm ok,let me try something quickly | 19:29 |
| @HeikoS | iglesiasg: but lets take the conversion case | 19:29 |
| iglesiasg | HeikoS, https://en.cppreference.com/w/cpp/language/dynamic_cast | 19:30 |
| @HeikoS | iglesiasg: like MCLabels and BinaryLabels | 19:30 |
| iglesiasg | but just check the first line there | 19:30 |
| iglesiasg | converts classes up, down, .... in the class hierarchy | 19:30 |
| @HeikoS | iglesiasg: but here we have DenseLabels instance | 19:30 |
| @HeikoS | it is not a DenseLabels pointer to a BinaryLabels instance | 19:30 |
| iglesiasg | ok, I think I understand | 19:31 |
| @HeikoS | the object itself it DenseLabels | 19:31 |
| @HeikoS | but maybe lets take the conversion | 19:31 |
| @HeikoS | we have multiclass labels for some reason | 19:31 |
| iglesiasg | that will require creating a new object for sure then | 19:31 |
| @HeikoS | ok | 19:31 |
| iglesiasg | if you want a BinaryLabels | 19:31 |
| @HeikoS | now | 19:31 |
| @HeikoS | if my parameter is const CLabels* | 19:32 |
| @HeikoS | I can of course cast this to const CBinaryLabels* | 19:32 |
| @HeikoS | (if possible) | 19:32 |
| iglesiasg | ok | 19:32 |
| @HeikoS | so my return type in that case would be: const CBinaryLabels* | 19:32 |
| @HeikoS | but if I need to convert, i need to create a new instance | 19:32 |
| @HeikoS | so what is my return type then? | 19:33 |
| iglesiasg | also const BinaryLabels? | 19:33 |
| @HeikoS | ok cool | 19:33 |
| @HeikoS | now my question is | 19:33 |
| @HeikoS | how does this work? :) | 19:33 |
| @HeikoS | auto result = new CBinaryLabels(); convert; return result; | 19:34 |
| @HeikoS | and the return type is const CBinaryLabels* | 19:34 |
| sukey1 | [https://github.com/shogun-toolbox/shogun] Pull Request https://github.com/shogun-toolbox/shogun/pull/4281 opened by shubham808 | 19:34 |
| @HeikoS | now who destroys the object | 19:34 |
| @HeikoS | ? | 19:34 |
| iglesiasg | I guess you should do | 19:34 |
| iglesiasg | const auto* result = new... | 19:34 |
| @HeikoS | who owns it? | 19:34 |
| iglesiasg | in this case the caller | 19:35 |
| iglesiasg | this would be old-school memory management | 19:35 |
| @HeikoS | I would say so | 19:35 |
| @HeikoS | but now | 19:35 |
| @HeikoS | the caller cannot call SG_UNREF | 19:35 |
| iglesiasg | but this is "bad nowadays" | 19:35 |
| iglesiasg | it should be in a smart pointer preferably | 19:35 |
| @HeikoS | because that modifies the object | 19:35 |
| iglesiasg | bad practice let's say not just bad | 19:35 |
| iglesiasg | aha! That's true lol | 19:36 |
| @HeikoS | so is it possible to offer a method like that even? | 19:36 |
| iglesiasg | I guess this could be listed in a smart pointer design discussion | 19:36 |
| iglesiasg | in the sense of discussing the approaches | 19:37 |
| iglesiasg | the reference counter is part of the object | 19:37 |
| iglesiasg | or the object is part of the reference counter | 19:37 |
| iglesiasg | but let's think about this | 19:37 |
| @HeikoS | yes | 19:37 |
| @HeikoS | I am blocked by this atm | 19:37 |
| iglesiasg | I don't see it that clear now | 19:37 |
| iglesiasg | so | 19:38 |
| @HeikoS | I wrote all those conversion things and now ran across a const CLabels* pointer, and now it seems screwed | 19:38 |
| iglesiasg | when we have a const CBinaryLabels* this means that the CBinaryLabels cannot be modified | 19:38 |
| iglesiasg | the pointer can be reassigned | 19:38 |
| iglesiasg | agreed? | 19:38 |
| @HeikoS | y | 19:38 |
| iglesiasg | and the problem is of course that the ref count is part of the sgobject and therefore part of the binary labels as well | 19:39 |
| @HeikoS | y | 19:39 |
| iglesiasg | so yeah, ok | 19:40 |
| iglesiasg | with more confidence now | 19:40 |
| iglesiasg | but I think we are in the same step as 4 minutes ago when I ahad! xD | 19:40 |
| @HeikoS | it seems to me that a reference counter inside objects basically disabled any constness | 19:40 |
| @HeikoS | but now I have a problem | 19:41 |
| @HeikoS | because | 19:41 |
| iglesiasg | ok one second | 19:41 |
| @HeikoS | I can either have the parameter of the binary_labels method be non-const (then all this works, casting and creating new) | 19:41 |
| @HeikoS | but then I cannot call it from a const pointer | 19:41 |
| @HeikoS | const pointer I HAVE to cast, I cannot convert | 19:41 |
| iglesiasg | but what does it actually happen if you try to compile some code that has a const SGObject*? | 19:42 |
| iglesiasg | does it really not compile at all?! | 19:42 |
| @HeikoS | no it works for example as modifier in a method that accepts a const SGObject* | 19:43 |
| iglesiasg | because what I am thinking is that after all ref and unref in SGObject are non-const methods | 19:43 |
| @HeikoS | but not sure whether it is possible to instantiate a const SGO | 19:43 |
| iglesiasg | exactly | 19:43 |
| iglesiasg | I think you will be able to do so | 19:43 |
| @HeikoS | I can never ref count it | 19:43 |
| iglesiasg | you just won't be able to call non-const methods | 19:43 |
| iglesiasg | not via the REF and UNREF method, no | 19:44 |
| @HeikoS | not at all actually | 19:44 |
| iglesiasg | because those after all fall back into calling ref unref methods, no? | 19:44 |
| @HeikoS | can't we make there counter mutable? | 19:44 |
| iglesiasg | maybe there are tricks to do that | 19:45 |
| iglesiasg | making part of a const instance non-const | 19:45 |
| iglesiasg | I don't remember anything right now about it | 19:45 |
| iglesiasg | but I think there are casting operators or rules for capturing types in templates that in a way "non-const" things | 19:45 |
| @HeikoS | https://stackoverflow.com/questions/105014/does-the-mutable-keyword-have-any-purpose-other-than-allowing-the-variable-to?utm_medium=organic&utm_source=google_rich_qa&utm_campaign=google_rich_qa | 19:46 |
| iglesiasg | so you give it something that is const and/or volatile and it gets rid of those qualifiers | 19:46 |
| @HeikoS | I have no experience with it | 19:46 |
| iglesiasg | wtf is this bitwise and logical const? xD | 19:47 |
| iglesiasg | but yeah | 19:47 |
| @HeikoS | lol :D | 19:47 |
| @HeikoS | ok so bad news for my conversion method | 19:47 |
| iglesiasg | from the second answer and the question itself, it looks like mutable can come to rescue here | 19:47 |
| @HeikoS | not possible to create new things and return const pointer to them and pass ownership | 19:48 |
| iglesiasg | you sure? :) | 19:48 |
| @HeikoS | lisitsyn, wiking can we make the reference counter mutable? ^ | 19:48 |
| @HeikoS | iglesiasg: no :) | 19:48 |
| iglesiasg | let's make it before they answer xD | 19:48 |
| iglesiasg | just kidding | 19:48 |
| @HeikoS | hehe | 19:49 |
| @HeikoS | seems like it might help | 19:49 |
| @HeikoS | then we can make ref() method const | 19:50 |
| @HeikoS | uh | 19:50 |
| @HeikoS | feels weird | 19:50 |
| iglesiasg | fml Some::ref calls T::ref hehehe | 19:50 |
| iglesiasg | but let's think about this | 19:51 |
| iglesiasg | does your conversion function need to be aware of ownership? | 19:51 |
| @HeikoS | it doesnt care | 19:51 |
| @HeikoS | but it might return the same thing that the caller gave, or not | 19:51 |
| iglesiasg | can it just work with T* and assume that data T will always be there? | 19:51 |
| iglesiasg | ok | 19:52 |
| iglesiasg | that's fine then, no? | 19:52 |
| @wiking | HeikoS, that's when lisitsyn said - and i agree - that it's playing with a shotgun | 19:52 |
| iglesiasg | yes, it definitely sounds like it could get dangerous if not under control | 19:53 |
| @wiking | hence the argument of not doing Some<const T> as it really doesnt make much sense :P just pass const :) | 19:54 |
| iglesiasg | HeikoS, so, the conversion function can take a const T* and return and S* | 19:54 |
| iglesiasg | HeikoS, where is the pitfall in that view? | 19:54 |
| iglesiasg | wiking, que no haha why is that the same? | 19:54 |
| @HeikoS | how do you mean that? | 19:54 |
| iglesiasg | what hence | 19:54 |
| @wiking | iglesiasg, since you have a problem with the ref counter etc | 19:54 |
| @wiking | just dont assume that you get ownership | 19:55 |
| @wiking | in this case of course it means that you need to copy | 19:55 |
| @HeikoS | iglesiasg: I think the problem is indeed ownership here | 19:55 |
| iglesiasg | wiking, oh ok sorry I was thinking still about the problem in general in the sense of shared ownership and const | 19:55 |
| @HeikoS | since casting doesnt change ownership, but creating a new instance does (the caller gets ownership) | 19:55 |
| iglesiasg | yeah, for sure. Copy or moved | 19:55 |
| iglesiasg | it is a new object being created after all | 19:56 |
| @wiking | iglesiasg, moving is fine as well :) | 19:56 |
| @HeikoS | so the binary_labels method mixes ownership semantics | 19:56 |
| @HeikoS | wiking: you have thoughts on this? | 19:56 |
| @wiking | HeikoS, move | 19:56 |
| @wiking | i would simply move | 19:56 |
| @wiking | everything | 19:56 |
| iglesiasg | queen to d4 | 19:57 |
| @wiking | question is whether you need the original stuff | 19:57 |
| @HeikoS | you do | 19:57 |
| @wiking | where? | 19:57 |
| @wiking | in sthe swig? | 19:57 |
| @HeikoS | it is a member of the CMachine | 19:57 |
| @wiking | label/ | 19:57 |
| @wiking | ? | 19:57 |
| @wiking | i mean in the long run we wanna drop that 'feature' | 19:57 |
| @wiking | no? :() | 19:57 |
| @wiking | :) | 19:57 |
| @HeikoS | dont understand what you mean | 19:58 |
| @wiking | it is a member of the CMachine | 19:58 |
| @wiking | the clabels? | 19:58 |
| @HeikoS | yeah or features | 19:58 |
| @wiking | in both cases | 19:58 |
| @wiking | do you think that it makes sense | 19:58 |
| iglesiasg | HeikoS, binary_labels function always creates new Some, no? | 19:58 |
| @wiking | to have ownership of those? | 19:58 |
| @HeikoS | wiking: no | 19:58 |
| @HeikoS | i see | 19:58 |
| @HeikoS | iglesiasg: currently, yes | 19:58 |
| @HeikoS | but it doesnt accept a const arugment so that is fine | 19:59 |
| @wiking | i mean i really dont see why is it a good design acutally | 19:59 |
| @wiking | do to CMachine(Features, Labels) | 19:59 |
| iglesiasg | why is one some small and the other Some big, I get confused :( | 19:59 |
| @HeikoS | Some is the class | 19:59 |
| @HeikoS | and some is a method to create a Some | 19:59 |
| @wiking | HeikoS, template :P | 19:59 |
| @wiking | but yeah | 19:59 |
| @HeikoS | function, not method | 19:59 |
| iglesiasg | ok, a make_some | 19:59 |
| @wiking | iglesiasg, ye | 19:59 |
| @wiking | does anybody have a good use-case for CMachine(Features, Labels) | 20:00 |
| @wiking | ? | 20:00 |
| iglesiasg | good good | 20:00 |
| @HeikoS | idk | 20:00 |
| @wiking | because imo | 20:00 |
| @wiking | it's more confusing | 20:00 |
| @wiking | than actually helping | 20:00 |
| @wiking | see CMachine(Features, Labels) | 20:00 |
| @wiking | and then .train(Features) | 20:00 |
| @wiking | :P | 20:00 |
| @wiking | wtf is that :D | 20:00 |
| iglesiasg | yeahhhh it is odd | 20:00 |
| @wiking | and then if you want new Features | 20:01 |
| @HeikoS | thats just old ctors | 20:01 |
| iglesiasg | I think being restrictive here in the API could help | 20:01 |
| @HeikoS | its a shit this | 20:01 |
| @wiking | then you need to | 20:01 |
| @HeikoS | but thats all independent | 20:01 |
| @wiking | machine.set_labels() | 20:01 |
| @wiking | right? | 20:01 |
| @HeikoS | wiking: yes | 20:01 |
| @wiking | i mean yes it's independent | 20:01 |
| @wiking | but say we get rid of owning these stuff | 20:01 |
| @wiking | because we agree that this design | 20:01 |
| @wiking | is just not good | 20:01 |
| @HeikoS | ok so what you are saying is that | 20:01 |
| @HeikoS | we want to just pass const Cfeatures* in the train method | 20:01 |
| @HeikoS | and that's it | 20:02 |
| @wiking | const Features, const Labels | 20:02 |
| @wiking | go be precise | 20:02 |
| iglesiasg | haha | 20:02 |
| @HeikoS | okok | 20:02 |
| @wiking | and you get back a CLabels | 20:02 |
| @wiking | but actually it should be const Clabels | 20:02 |
| @wiking | :P | 20:02 |
| @HeikoS | but now this converting thing is still a problem in the way it is done right now | 20:02 |
| @wiking | yeah | 20:02 |
| @wiking | but | 20:02 |
| @wiking | say you have this done | 20:02 |
| @HeikoS | we should kill it then | 20:02 |
| @wiking | machines are not owning labels&features | 20:03 |
| @HeikoS | and force the user to pass the correct type | 20:03 |
| @wiking | then you can actually start having a move pattern for binary_labels | 20:03 |
| @HeikoS | and in train we try to cast and if it doesnt work then bail | 20:03 |
| @wiking | and then you actually really just pass CLabels&& | 20:03 |
| @wiking | yeah i mean in train | 20:03 |
| @wiking | we always need to do a typecheck anyways | 20:03 |
| @HeikoS | yeah | 20:03 |
| @HeikoS | I just liked the conversion thingi | 20:04 |
| @HeikoS | [0,1,2... ] to [-1,1] for example | 20:04 |
| @wiking | yeah the conversion just needs changing a bit | 20:04 |
| @HeikoS | because that creates a new instance | 20:04 |
| @wiking | that can be changed with a move pattern | 20:04 |
| @wiking | or if you want then copy everything | 20:04 |
| @wiking | :) | 20:04 |
| @wiking | both should be fine | 20:04 |
| @wiking | the current way is a bit mix | 20:04 |
| @HeikoS | but then the method cannot return const pointer | 20:04 |
| @wiking | and that's the bad part | 20:04 |
| @wiking | which method? | 20:04 |
| @HeikoS | dispatcher | 20:04 |
| @HeikoS | inside CMachine::train | 20:05 |
| @HeikoS | because casting gives you a const pointer | 20:05 |
| @HeikoS | but converting gives you new obj, which cannot be a const poointer | 20:05 |
| @HeikoS | the idea of "either cast or if not possible create new object and convert" mixes ownership | 20:06 |
| @HeikoS | since second case gives ownership to caller | 20:06 |
| @HeikoS | and first case doesnt | 20:06 |
| @wiking | well yeah :) except if you do a move ptr story in case of new object creation | 20:07 |
| @wiking | or? | 20:08 |
| @wiking | i mean in case it's casting | 20:08 |
| @HeikoS | I am unsure | 20:08 |
| @wiking | you are working on the same obj | 20:08 |
| @HeikoS | what about the caller of CMacihne::train | 20:08 |
| @wiking | so nothing happens | 20:08 |
| @HeikoS | he stil lthinks he owns the obj | 20:08 |
| @wiking | if you actually have to convert it not only cast it | 20:08 |
| @wiking | then actually drop the src and give a new object where you own the obj | 20:09 |
| @wiking | but | 20:09 |
| @wiking | since you should be passing | 20:09 |
| @wiking | const CFeatures, const CLabels | 20:09 |
| @HeikoS | thats annoying as well as most of the time, a cast is sufficient | 20:09 |
| @wiking | mmm | 20:09 |
| @wiking | where are the current usecases | 20:10 |
| @HeikoS | I mean it doesnt really matter | 20:10 |
| @wiking | of the binary_labels | 20:10 |
| @wiking | for example? | 20:10 |
| @HeikoS | as train is expensive anyways | 20:10 |
| @HeikoS | one usecase is examples | 20:10 |
| @HeikoS | there we load labels from a file | 20:10 |
| @HeikoS | and we dont know what those labels are | 20:10 |
| @HeikoS | mc or binary | 20:10 |
| @wiking | why? | 20:10 |
| @HeikoS | so we put them in CDenseLabels | 20:10 |
| @HeikoS | because all we get is a csv file | 20:10 |
| @wiking | yeah but | 20:11 |
| @wiking | :) | 20:11 |
| @wiking | i mean we agreed that this is a shit anyways | 20:11 |
| @wiking | anyhow yeah | 20:11 |
| @wiking | so you read the labels | 20:11 |
| @HeikoS | it is | 20:11 |
| @wiking | and then you try to cast it | 20:11 |
| @HeikoS | but thats how it currently is | 20:11 |
| @wiking | "cast" | 20:11 |
| @HeikoS | yes | 20:11 |
| @wiking | and that's a conversion | 20:11 |
| @HeikoS | and the "cast" | 20:11 |
| @wiking | right? | 20:11 |
| @HeikoS | just steals the vector | 20:11 |
| @HeikoS | and creates a new isntance | 20:11 |
| @wiking | mmm | 20:11 |
| @HeikoS | not steals | 20:11 |
| @HeikoS | assigns SGVector | 20:12 |
| @HeikoS | mmmh | 20:12 |
| @wiking | but shouldn't we actually try to read in | 20:12 |
| @wiking | MC | 20:12 |
| @wiking | in the first place | 20:12 |
| @HeikoS | could | 20:13 |
| @wiking | and while reading and creating | 20:13 |
| @wiking | you do the validation | 20:13 |
| @wiking | and if it's asdf | 20:13 |
| @wiking | then fuck you | 20:13 |
| @wiking | :) | 20:13 |
| @wiking | and you throw an exception | 20:13 |
| @HeikoS | there is just this thing that from a user API perspective | 20:13 |
| @HeikoS | there is really no sense to distringuish MC and binary labels | 20:13 |
| @wiking | yeah | 20:13 |
| @HeikoS | that was the idea | 20:13 |
| @HeikoS | so I was thinking at least we can have that in the API already | 20:13 |
| @wiking | i mean | 20:13 |
| @HeikoS | and then change internally at some point | 20:13 |
| @wiking | this is in case swig | 20:13 |
| @wiking | no? | 20:13 |
| @HeikoS | any user-facing | 20:14 |
| @wiking | mmmm | 20:14 |
| @wiking | in c++ i would agrue | 20:14 |
| @wiking | that it's better to be as specific as possible | 20:14 |
| @wiking | as soon as | 20:14 |
| @HeikoS | sure | 20:14 |
| @wiking | so that you actually throw exception | 20:14 |
| @wiking | not in train | 20:14 |
| @HeikoS | but even then | 20:14 |
| @wiking | but already when you wanna MC a clearly binary labeled input | 20:14 |
| @wiking | or i mean | 20:14 |
| @wiking | the other way round | 20:14 |
| @wiking | because Mc(binary ) should be fine | 20:15 |
| @HeikoS | yep | 20:15 |
| @wiking | but yeah of course | 20:15 |
| @wiking | in a python settings | 20:15 |
| @wiking | you really wanna be able to do | 20:16 |
| @wiking | m.train(X, y) | 20:16 |
| @wiking | and you shouldnt care too much about converting y to the right format | 20:16 |
| @wiking | etc etc | 20:16 |
| @HeikoS | exactly | 20:16 |
| @wiking | as i agree | 20:16 |
| @wiking | that having the current | 20:16 |
| @wiking | labels = BinaryLabels(v) | 20:16 |
| @wiking | is just annoying | 20:16 |
| @HeikoS | we could also do more gluing code | 20:17 |
| @HeikoS | i.e. always just cast inside the algos | 20:17 |
| @HeikoS | i.e. no conversion | 20:17 |
| @HeikoS | but then make swig code that converts | 20:17 |
| @HeikoS | based on the enums of the machine | 20:17 |
| @HeikoS | ah but thats not feasible to do | 20:18 |
| @HeikoS | as we first need train(const CFeatures, ..) to be the only place where things are passed | 20:18 |
| @HeikoS | otherwise we cannot glue efficiently | 20:18 |
| @HeikoS | chicken egg problem :( | 20:18 |
| @wiking | btw | 20:18 |
| @wiking | would the conversion | 20:18 |
| @wiking | of the old ctor style | 20:18 |
| @wiking | to a train(X, y) | 20:19 |
| @wiking | be that heavy? | 20:19 |
| @HeikoS | like inside ::train we do BinaryLabels(y) ? | 20:19 |
| @wiking | i mean really just to clear up this mess | 20:19 |
| @wiking | and remove the ownership | 20:19 |
| @wiking | of the labels/features | 20:19 |
| @HeikoS | yes I am tending towards this as well atm | 20:19 |
| @wiking | in the first place | 20:19 |
| @HeikoS | ah | 20:20 |
| @wiking | we all agree that that's a confusion concept | 20:20 |
| @HeikoS | sorry I see what you mean | 20:20 |
| @HeikoS | I think yes :( | 20:20 |
| @wiking | so how about first dropping that | 20:20 |
| @wiking | in a first round | 20:20 |
| @HeikoS | have to refactor a lot of code | 20:20 |
| @HeikoS | as a lot of code is accessing m_labels | 20:20 |
| @HeikoS | and m_features | 20:20 |
| @HeikoS | via helper methods | 20:20 |
| @wiking | mm | 20:20 |
| @HeikoS | and there is some open design qs as well | 20:20 |
| @HeikoS | like kernel machine | 20:20 |
| @HeikoS | needs to keep a reference to features | 20:21 |
| @HeikoS | does it copy? | 20:21 |
| @HeikoS | or does it only copy the model features (like store_model_features?) | 20:21 |
| @HeikoS | or what? | 20:21 |
| @HeikoS | so not that clear | 20:21 |
| @HeikoS | I think what I will do for now (because I want to move on) | 20:21 |
| @HeikoS | is to create new instances in the conversion method, and then assign the members | 20:22 |
| @HeikoS | and then once we have a proper labels/features managment | 20:22 |
| @wiking | who uses fatures | 20:22 |
| @HeikoS | we can change that | 20:22 |
| @wiking | just tried checking in kernel machine | 20:22 |
| @HeikoS | in a minimal way | 20:22 |
| @HeikoS | apply | 20:22 |
| @HeikoS | apply uses the features | 20:22 |
| @HeikoS | every kernel machine | 20:22 |
| @HeikoS | and all gps | 20:22 |
| @HeikoS | as you need the kernel between training and test data | 20:22 |
| @HeikoS | which you dont know at training time because you dont know the test data | 20:23 |
| @HeikoS | linear models dont need features | 20:23 |
| @HeikoS | kernel machines dont need labels | 20:23 |
| @HeikoS | actually no point in ever storing labels outside of train | 20:23 |
| @HeikoS | ok I will go now | 20:24 |
| @HeikoS | wiking: see you later! | 20:24 |
| @wiking | ttyl | 20:24 |
| @wiking | starting with labels | 20:25 |
| @wiking | with be a good idea actually | 20:25 |
| @wiking | just to drop that | 20:25 |
| iglesiasg | I think I understand now what wiking was saying before when we were discussing | 20:28 |
| iglesiasg | so not about shared ownership in general but in Shogun, in particular the pattern that when a SGObject has a member of another SGObject generally it is owning and ref counted | 20:29 |
| iglesiasg | and yep I agree with that | 20:30 |
| iglesiasg | in general not clear why in e.g ?m = KNN() // a machine instance; m.train(features, labels)? the machine must own the features and labels | 20:31 |
| iglesiasg | in a way it makes it less restrictive | 20:32 |
| iglesiasg | but not sure if it is eventually worth | 20:32 |
| -!- HeikoS [~heiko@untrust-out.swc.ucl.ac.uk] has quit [Ping timeout: 255 seconds] | 20:35 | |
| @wiking | iglesiasg, loco mode on: https://www.facebook.com/notes/protect-the-graph/pyre-fast-type-checking-for-python/2048520695388071/ | 20:43 |
| iglesiasg | haha | 20:46 |
| iglesiasg | making Python more C++, nice | 20:47 |
| iglesiasg | hahaha | 20:47 |
| iglesiasg | but yeah, nice after all | 20:49 |
| iglesiasg | it will force people to write better code | 20:49 |
| iglesiasg | wiking, I started listening to a c++ podcast on the bike to and from job | 20:51 |
| @wiking | ah cppcast? | 20:51 |
| lisitsyn | fsck! | 20:51 |
| @wiking | its good | 20:51 |
| @wiking | i like that | 20:51 |
| iglesiasg | yeah, it is good | 20:51 |
| iglesiasg | two of the episodes I have listened to I really like | 20:51 |
| lisitsyn | wiking: ok I actually start to agree const T* is just fine | 20:52 |
| iglesiasg | yeah CppCast | 20:52 |
| iglesiasg | http://cppcast.com/2017/12/nicole-mazzuca/ | 20:53 |
| iglesiasg | very interesting from this guy Nicole Mazzuca | 20:53 |
| iglesiasg | it looked like he talked about similar stuff in CppCon, there?s a Youtube video | 20:54 |
| iglesiasg | but video dangerous with the bike lol | 20:54 |
| @wiking | https://youtu.be/dkngdqDwfEo?t=267 | 20:58 |
| @wiking | :> | 20:58 |
| iglesiasg | poor leoncitos | 21:11 |
| iglesiasg | :( | 21:11 |
| iglesiasg | and the other animals too | 21:11 |
| iglesiasg | haha Trevor Noah appeared in suggested videos, my evening is lost now :P | 21:11 |
| @wiking | :> | 21:21 |
| @wiking | iglesiasg, https://www.youtube.com/watch?v=cODmIxsna0g | 21:37 |
| @wiking | ;) | 21:37 |
| @wiking | watch this it's only 24 minutes :P | 21:38 |
| -!- iglesiasg [~iglesias@f119189.upc-f.chello.nl] has quit [Ping timeout: 256 seconds] | 21:39 | |
| -!- iglesiasg [~iglesias@f119189.upc-f.chello.nl] has joined #shogun | 21:43 | |
| -!- iglesiasg [~iglesias@f119189.upc-f.chello.nl] has quit [Ping timeout: 246 seconds] | 22:02 | |
| Trixis | >= | 22:27 |
| -!- HeikoS [~heiko@host86-146-49-221.range86-146.btcentralplus.com] has joined #shogun | 23:21 | |
| -!- mode/#shogun [+o HeikoS] by ChanServ | 23:22 | |
| sukey1 | [https://github.com/shogun-toolbox/shogun] Pull Request https://github.com/shogun-toolbox/shogun/pull/4281 merged by karlnapf | 23:31 |
| sukey1 | [https://github.com/shogun-toolbox/shogun] New commit https://github.com/shogun-toolbox/shogun/commit/02ee9a54554a8b6a50f502f58ffe1c05a0a66a32 by karlnapf | 23:31 |
| --- Log closed Sat May 12 00:00:03 2018 | ||
Generated by irclog2html.py 2.10.0 by Marius Gedminas - find it at mg.pov.lt!