Closed EricHanLiu closed 6 years ago
It seems that we are working under the assumption that we will always work with three layers. What about if we want to perform a classification task as foreground/background (i.e., binarization)?
Yes, in the linked issue I described a bit why the 3 layers are hardcoded. It seems difficult to variablize the number of input/output ports for a Rodan job, and change this dynamically while the user is using Pixel (especially with inputs, the inputs have to be fixed in advance and determined by the # of CM layers).
Jorge said he was unsure how to do this for the CM, which precedes Pixel's port quantity determination, and I'm not sure if it's a design implementation that Rodan doesn't want to allow.
Maybe if we had a fixed number of optional ports and just ensured that there is consistency over every job, then that could work as an alternative?
In general, it seems difficult to be able to retrieve the number of input layers or required output layers from within Pixel.js, which is necessary since layers have to be created in advance to work properly
Besides the assumption pointed out by @vigliensoni, the code looks fine. Could you elaborate a bit on what are the string literals you are looking for in your python script? Particularly, the one for activation: this.uiManager.createPluginElements(this.layers);
In which context should this string appear in the javascript file? My concern, are you sure we won't be injecting activation snippets all over the place with future changes to Pixel.js?
I made a search for that string in the code of this branch, I found several instances in ./static/js/pixel.js
, but that is not the file you are opening in python, I am guessing you are accessing the "compiled" file after you build, right? Just wanna be convinced it makes sense to search for this particular string and inject code on it.
@napulen
In general, I look for specific string literals to know where to insert the wrapper code. I've tried to pick the best places so that even through dramatic changes to the source code, these inserts would still be in the appropriate places. For example, the import_code
searches for the first import {
statement in pixel.js
and inserts it before this.
The string literal you refer to for activation is searched for so that the wrapper can be activated right before the pluginElements are created (the Pixel UI), this ensures the submitToRodan button appears better formatted.
The this.uiManager.createPluginElements(this.layers);
occurs multiple times in the source code, but it first occurs when activatePlugin()
is called. I have a flag which, after the code has been added once, prevents future injections (so it ensures at most 1 line added for each method).
There are also several instances in the source/pixel.js
file, but I believe since it's a submodule you won't be able to find it until the submodule has been initialized and updated.
I am accessing the non-compiled file in order to add these lines.
Right, missed your flags. Cool. I am okay with the merge if @vigliensoni agrees.
The main thing to do I guess is to generalize the number of input/output ports, but I'm not sure if that is a trivial task (and should probably belong on another branch). If Rodan can pass how many optional ports were activated into the job, then we could probably generalize the number of layers within Pixel.js from this number.
Seems like it'd make more sense to just allow 5-8 output ports, all optional, which Pixel reads and forces the user to create that many layers before exporting (otherwise Rodan throws an exception). This also depends on @calvozaragoza's CM working with a dynamic number of output ports, and Pixel.js would read this number and create that many layers from the start.
The number of layers is not fixed, but variable. For example, in the Salzinnes manuscript, we may want to segment the document in neumes, text, and stafflines. But in St. Gall (staffless manuscript), we may only need two classes.
If we don't have a clear strategy for tackling this now, I would merge this version and leave this issue for further improvement.
On Tue, May 29, 2018 at 3:57 PM, Eric Han Liu notifications@github.com wrote:
The main thing to do I guess is to generalize the number of input/output ports, but I'm not sure if that is a trivial task (and should probably belong on another branch). If Rodan can pass how many optional ports were activated into the job, then we could probably generalize the number of layers within Pixel.js from this number.
Seems like it'd make more sense to just allow 5-8 output ports, all optional, which Pixel reads and forces the user to create that many layers before exporting (otherwise Rodan throws an exception). This also depends on @calvozaragoza https://github.com/calvozaragoza's CM working with a number of output ports, where Pixel.js would need to know this number and create this many layers from the start.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/DDMAL/pixel_wrapper/pull/23#issuecomment-392922562, or mute the thread https://github.com/notifications/unsubscribe-auth/AAeCxX5VYPWSee8MS6R9KM5vP5yrU3o6ks5t3agNgaJpZM4USC4X .
I agree, in which case we'd need to know this number of layers in advance by specifying the number of input/output ports, since Rodan crashes if it doesn't receive the exact number of output files specified by the ports.
Also, the number of input and output ports needs to be the same (but we can't enforce that in Rodan so it has to be user-side). So if there's a way to get this number of ports from Rodan and pass that into Pixel.js, then it'd be easy to specify how many layers the user must create and classify (and the default number of layers created).
I think it's best to merge this version, and then in another branch work on variablizing the default/required number of layers based on the number of input & output ports in the workflow
Pixel.js has now been successfully implemented as a git submodule, and the README instructions have been updated with the new commands required to make the magic happen (updating/loading the submodule and activating the wrapper).
See the accompanying issue.