Plugin import keras #120
Plugin import keras #120
Conversation
|
Actually, upon testing it a bit further, I found a couple bugs around parsing and setting attributes. Essentially, I imported the |
| ImportKeras.prototype._toPythonIterable = function (obj) { | ||
| if (obj == null) { | ||
| return 'None'; | ||
| } | ||
| if (obj instanceof Array) { | ||
| return '[' + obj.map((val) => { | ||
| return this._toPythonIterable(val); | ||
| }).join(', ') + ']'; | ||
| } else { | ||
| return obj; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Actually, upon testing it a bit further, I found a couple bugs around parsing and setting attributes. Essentially, I imported the
redshiftModel.jsonfile and then generated code to ensure that it was valid keras and encountered the following error:NameError: name 'false' not defined
I think I know the cause of the problem you alluded to, booleans start with capital case in python which I had not considered here. Refactoring this to the following should do the job.
// This method is used to convert javascript arrays/booleans to a
// list(python)/boolean in string Representation. Needed for
// Code generation.
ImportKeras.prototype._toPythonType = function (obj) {
if (obj == null) {
return 'None';
}
if (obj instanceof Array) {
return '[' + obj.map((val) => {
return this._toPythonType(val);
}).join(', ') + ']';
} else if (typeof obj === 'boolean') {
return obj ? 'True' : 'False';
} else {
return obj;
}
};There was a problem hiding this comment.
On a minor side note, it might make sense to test GenerateKeras on this plugin as a test.
There was a problem hiding this comment.
Yep. I think it is also expecting tuples ((1,2,3)) rather than arrays which also caused an error after fixing the booleans. However, this is something that might be better to fix in GenerateKeras (https://github.com/deepforge-dev/deepforge-keras/blob/master/src/plugins/GenerateKeras/GenerateKeras.js#L416)
…3804), change variables scope (Adress #120#discussion_r338052173)
|
|
|
The problem with #120 (comment) was that the layer pointers were not being correctly set. Also, some metanodes in |
… methods, change to l1_l2 in class maps
…models without dropout layers)
…models without dropout layers)
|
This adds the test to run code generated from the imported architectures. However, the |
|
I am just seeing the comment about case-consistency now. I actually wouldn't consider it an inconsistency with the metamodel; layers are often aliased in keras and deepforge-keras simply creates one and maps all the aliases to the created one (the aliases are recorded in the json schemas). Rather than hard coding the aliases again, it would be better to resolve them using the schemas. That said, there is an issue around this as it appears that the aliases are being picked up as the actual regularizers and the actual names are being ignored (schema vs source). |
|
So the bug with the dropout layers is due to not checking what input/output to use when connecting the layers. Currently, the first port is selected (from an unordered set). I added a helper method which returns the ordered set of inputs/outputs (the ports can be ordered using the However, are there any cases where we will want to connect the layers using some other input? It is possible to connect layers to other inputs but it doesn't seem like this is addressed in the code. That said, it is coming along! Nice work adding the integration testing capabilities - I am happy they have helped us catch these sorts of issues early! |
To address #12,
A new plugin called
ImportKerasis added that takes in akerasmodel as a JSON file and tires to convert it to adeepforgeresource.Files Changed:
Add gitattributes to the root dir forAdd .gitattributes file. #123LF/CRLFFix.Add IDE Specific paths toAdd .gitattributes file. #123.gitignore.