-
-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
added list initialisation #4606
Conversation
@@ -12,7 +12,6 @@ | |||
"InitialSeperatorWhenArgs>0": false | |||
}, | |||
"string": "$name = $arguments", | |||
"CharVector": "$name = zeros(1, $arguments, 'char')", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't work in octave (4.4.1)
"RealVector": "$name = NArray['float', [$arguments])", | ||
"LongRealVector": "$name = NArray['float', [$arguments])", | ||
"ComplexVector": "NArray['scomplex', [$arguments])" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this out with the most recent numo/NArray version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah actually, what type are the zero constructors? I can't find it anywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, this was when I was confused as to how to construct with a list of args in ruby. was simpler than I thought! :D
# custom SGType construction | ||
if typeKey == "ShogunSGType"\ | ||
and init[0][typeKey] in self.targetDict["ListInit"]: | ||
template = Template(self.targetDict["ListInit"][init[0][typeKey]]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be moved out of the condition. This substitution only works if there is an implementation of the list initialiser of the target language
@@ -38,6 +38,18 @@ | |||
"RealMatrix": "DoubleMatrix $name = new DoubleMatrix($arguments)", | |||
"LongRealMatrix": "DoubleMatrix $name = new DoubleMatrix($arguments)" | |||
}, | |||
"ListInit": { | |||
"BoolVector": "BoolVector $name = new BoolVector({$arguments})", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure how to do this one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t know either, last resort would be to translate it to explicit assignment although that would suck
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like it! I guess now it’s done work to make it work with all the langs...
It would be cool if you could also try to make it work with R and ruby. While those meta examples are currently disabled in the build, they in principle work (r doesn’t have static calls, ruby doesn’t have overloaded methods/functions) since both support list inits. I think it would be sufficient to look at generates output and locally try it that works
OK, I can have a look into R. With ruby I am not sure how it would work, as the old NArray doesn't seem to have a list initialiser? The new NArray library does seem to have it though. But I am not sure which one shogun supports? OLD: https://github.com/masa16/narray I figured out how to make it work, but there is no support for bools in ruby's old NArray library, so it might be worth upgrading to numo-array |
@@ -34,10 +34,22 @@ | |||
"IntMatrix": "DoubleMatrix $name = new DoubleMatrix($arguments)", | |||
"LongIntMatrix": "DoubleMatrix $name = new DoubleMatrix($arguments)", | |||
"ULongIntMatrix": "DoubleMatrix $name = new DoubleMatrix($arguments)", | |||
"ShortRealMatrix": "DoubleMatrix $name = new DoubleMatrix($arguments)", | |||
"ShortRealMatrix": "FloatMatrix $name = new FloatMatrix($arguments)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am assuming this should be FloatMatrix
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what the typemap exactly does...but I think our current Java bindings treat everything as double matrix. Might be worth cleaning that up at some point soon
Shogun still uses the old one and we have an issue for upgrading here. It is some work as the typemaps will have to be re-written. That is cumbersome as ruby is currently disabled due to #4177. |
I am thinking that maybe I skip boolean for now? There is still some lack of support for it in java, ruby and r (at least) so can address that separately? |
@@ -1,5 +1,3 @@ | |||
BoolVector bool_vector([True, False]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok!
Soooo merge? :) |
Yup, seems fine to me. When there is a need I can add matrix initialisation |
* added list initialisation * minor fixes * numo narray * fix floating point * removed boolean list init for now
list initialisation to replace starting empty vector and the replace elements