| --- Log opened Thu Aug 09 00:00:45 2018 | ||
| -!- HeikoS [~heiko@2a00:23c5:e10a:5c00:3078:e3f2:a766:5a01] has joined #shogun | 00:45 | |
| -!- mode/#shogun [+o HeikoS] by ChanServ | 00:45 | |
| -!- HeikoS [~heiko@2a00:23c5:e10a:5c00:3078:e3f2:a766:5a01] has quit [Ping timeout: 256 seconds] | 01:02 | |
| @sukey | [https://github.com/shogun-toolbox/shogun] Pull Request https://github.com/shogun-toolbox/shogun/pull/4390 synchronized by vinx13 | 04:16 |
|---|---|---|
| -!- sukey [~nodebot@ks312251.kimsufi.com] has quit [Read error: Connection reset by peer] | 08:48 | |
| -!- sukey [~nodebot@ks312251.kimsufi.com] has joined #shogun | 08:49 | |
| -!- mode/#shogun [+o sukey] by ChanServ | 08:49 | |
| -!- HeikoS [~heiko@2a00:23c5:e10a:5c00:3078:e3f2:a766:5a01] has joined #shogun | 10:46 | |
| -!- mode/#shogun [+o HeikoS] by ChanServ | 10:46 | |
| wuwei[m] | HeikoS: hi | 10:48 |
| @HeikoS | hi | 10:48 |
| @HeikoS | wuwei[m]: how are things? | 10:48 |
| wuwei[m] | i will finish peer review today | 10:48 |
| wuwei[m] | but now i have a problem with pipeline xval meta | 10:48 |
| @HeikoS | wuwei[m]: could you prioritise the peer review over the problem? | 10:49 |
| @HeikoS | since shubham then can do things in parallel | 10:49 |
| wuwei[m] | sure | 10:49 |
| @HeikoS | wuwei[m]: ping me once you are done, then maybe I can help with the problem with xval | 10:50 |
| @HeikoS | wuwei[m]: what problem do you have? | 10:50 |
| wuwei[m] | the problem is that i can't put a Some<CPipeline> into CMachine* | 10:50 |
| wuwei[m] | if the pipeline builder returns Some<CMachine>, then Some can be implicitly cast to raw pointer | 10:51 |
| @HeikoS | yeah | 10:51 |
| @HeikoS | we decided to only have those base types in the outside API in meta examples | 10:52 |
| @HeikoS | this is one of the reasons why I suggested that this type is transparent | 10:52 |
| @HeikoS | ie return Machine | 10:52 |
| @HeikoS | and then c++ users can cast it if they need the interface | 10:52 |
| @HeikoS | and we can add a factory for casting from swig | 10:53 |
| @HeikoS | CPipeline* pipeline(CMachine*) | 10:53 |
| @HeikoS | in factory.h | 10:53 |
| @HeikoS | wuwei[m]: see what I mean> | 10:53 |
| @HeikoS | ? | 10:53 |
| wuwei[m] | yeah this should work | 10:53 |
| @HeikoS | it is a bit ugly as we are forcing C++ users to use .as if they want the Pipeline API | 10:54 |
| @HeikoS | but I dont see another way of making this work with the examples | 10:55 |
| @HeikoS | you could add two finalizing methods maybe | 10:55 |
| @HeikoS | see shogun.i | 10:55 |
| @HeikoS | where we define methods just for the swig interface | 10:55 |
| @HeikoS | but here I would see something like | 10:55 |
| wuwei[m] | can we add an overload of put, that accepts Some<T>? | 10:55 |
| @HeikoS | we have that already | 10:56 |
| @HeikoS | but you cannot convert Some<Subclass> to Some<Baseclass> | 10:56 |
| @HeikoS | you can add two pipeline builder finalisers | 10:56 |
| @HeikoS | one that returns CPipeline (hidden from swig) | 10:56 |
| @HeikoS | and one that returns CMachine | 10:57 |
| @wiking | HeikoS, why hide it from swig? | 10:57 |
| @wiking | i mean if you hide it from swig | 10:57 |
| @HeikoS | actually it doesnt have to be hidden | 10:57 |
| @HeikoS | true | 10:57 |
| @wiking | there's really no need to actually have it as CPipeline | 10:57 |
| @HeikoS | then swig peeps can also get the API | 10:57 |
| @HeikoS | to access machines | 10:57 |
| @HeikoS | true | 10:57 |
| @HeikoS | so just two methods for now | 10:57 |
| @HeikoS | until we can regiuster the parameters of this thing | 10:58 |
| @HeikoS | then we can remove one | 10:58 |
| wuwei[m] | i mean the overload of put that accepts Some<T> can explicitly take the raw pointer, and then forward to put(T*) | 10:58 |
| @HeikoS | wuwei[m]: that is there already | 10:59 |
| @HeikoS | void put(const std::string& name, Some<T> value) | 10:59 |
| @HeikoS | { | 10:59 |
| @HeikoS | put(name, value.get()); | 10:59 |
| @HeikoS | } | 10:59 |
| wuwei[m] | ah didnt see this | 11:00 |
| wuwei[m] | but i wonder why T* cannot be put to base* | 11:01 |
| @HeikoS | smart pointer problem | 11:01 |
| @HeikoS | ah | 11:02 |
| @HeikoS | no | 11:02 |
| @HeikoS | template <class T, class = typename std::enable_if_t<is_sg_base<T>::value>> | 11:02 |
| @HeikoS | the trait | 11:02 |
| wuwei[m] | i see | 11:02 |
| @HeikoS | wuwei[m]: also put is type ultra safve | 11:02 |
| @HeikoS | if something is registered as CMachine | 11:02 |
| @HeikoS | you cannot put CMachineImplementation | 11:02 |
| @HeikoS | you have to call put<CMachine> | 11:03 |
| @HeikoS | this is why we cannot do the downcasting with Some and put | 11:03 |
| @HeikoS | I mean | 11:04 |
| @HeikoS | we can add an exception | 11:04 |
| @HeikoS | of explicitly allowing a put for CPipeline | 11:04 |
| @HeikoS | that then forwards it to put<CMachine> | 11:04 |
| @HeikoS | we would do this in shogun.i | 11:04 |
| @HeikoS | where we can overload the C++ classes | 11:04 |
| @HeikoS | this would in fact be a workaround that doesnt require our C++ api to change | 11:05 |
| @HeikoS | wiking: what do you think? | 11:05 |
| @HeikoS | then the pipeline builder returns Some<CPipelein> | 11:05 |
| wuwei[m] | sure, i will check that | 11:05 |
| @HeikoS | and we add SObject::put(Some<CPipeline>) inside the shogun.i | 11:05 |
| @HeikoS | hacky as fuck | 11:06 |
| @HeikoS | but cleaner for the C++ API | 11:06 |
| @HeikoS | ah no | 11:06 |
| @HeikoS | wont work with cpp meta examples I think | 11:06 |
| @HeikoS | no then the first option is better | 11:06 |
| wuwei[m] | ah yes | 11:06 |
| @HeikoS | pipline builder returns CMachine | 11:06 |
| @HeikoS | and another option for CPipeline | 11:06 |
| @HeikoS | wuwei[m]: btw | 11:07 |
| @HeikoS | did you try to register the paramerters of CPipeline? | 11:07 |
| @wiking | HeikoS, just a sec | 11:07 |
| @wiking | need to organize | 11:07 |
| @wiking | some other stuffg | 11:07 |
| @HeikoS | of course eq2uals and clone wont work | 11:07 |
| @HeikoS | but put/get might | 11:07 |
| @HeikoS | anyways | 11:07 |
| @HeikoS | gotta run | 11:08 |
| @HeikoS | wuwei[m]: pls make sure to send this peer review (draft) soon, so we can have a look. Would also be good if you put that into your blog later on | 11:08 |
| wuwei[m] | HeikoS: aren't parameters of pipeline, vector<pair<string, variant>> registerable? | 11:09 |
| @HeikoS | wuwei[m]: I think they might be | 11:10 |
| @HeikoS | gotta try | 11:10 |
| @HeikoS | just clone equals serialization wont work | 11:10 |
| @HeikoS | but those dont work anyways | 11:10 |
| @HeikoS | and then we can ask sergey to fix things with any :) | 11:10 |
| @HeikoS | lisitsyn: ^ | 11:10 |
| wuwei[m] | sure | 11:10 |
| @HeikoS | wuwei[m]: btw | 11:16 |
| @HeikoS | if you add the CMachine return type method for the pipeline builder | 11:16 |
| @HeikoS | you can still add a CPipeline* pipeline(CMachine*) method to factory.h | 11:17 |
| @HeikoS | in order to show how to use the CPipeline API in the meta examples | 11:17 |
| @wiking | HeikoS, jok just a sec i'll read back the logs | 11:20 |
| wuwei[m] | HeikoS: hi | 16:16 |
| wuwei[m] | https://docs.google.com/document/d/1gu3DdaFTfZB5TsDmE05qjz6yoOlwEb9szEOxkCW8H5o/edit?usp=sharing this is my peer review | 16:17 |
| @HeikoS | wuwei[m]: Id like to make a few comments, could you allow that for me? | 16:50 |
| wuwei[m] | sure | 16:50 |
| wuwei[m] | HeikoS: done, sorry didn't the email from google doc for permission request | 17:22 |
| wuwei[m] | *didn't see* | 17:23 |
| -!- HeikoS [~heiko@2a00:23c5:e10a:5c00:3078:e3f2:a766:5a01] has quit [Ping timeout: 256 seconds] | 19:52 | |
| -!- witness [uid10044@gateway/web/irccloud.com/x-eoccyknjkfodfuwe] has joined #shogun | 20:48 | |
| --- Log closed Fri Aug 10 00:00:47 2018 | ||
Generated by irclog2html.py 2.10.0 by Marius Gedminas - find it at mg.pov.lt!