VisBOL / visbol-js

SBOLv rendering in JavaScript
visbol.org
BSD 2-Clause "Simplified" License
10 stars 3 forks source link

Interactions #82

Closed asadeg02 closed 6 years ago

asadeg02 commented 6 years ago

Resolves #64 and #15. The functionality for rendering all 5 types of interactions (inhibition, production, simulation, non-covelant-binding, degradation) has been added. Now visbol can even handle files containing more than one interaction. if there are multiple interactions in a circuit, we render each interaction in a separate segment, so Now visbol is able to handle more than just simple files. Also displaylist in displaylist tab is updated with interaction lists. Readme has also been updated accordingly. Below is the rendering for each interaction.

Simple Inhibition simple-inhibition

Simple simulation (in this one we are not able to detect type of complex, that's why we are using SBGN) simple-simulation

Simple production simple-production

Degredation simple-degradation

Non-covelant-binding simple-non-covelant-biding

Interactions in circuits interaction-all

Complexes in interaction with circuit updated-activation

below is also an example of an interaction in interaction list of displaylist in diplaylist tab: { "displayId": "IPTG_LacI_protein_complex_formation", "name": "", "type": "non-covelant-binding", "SBO": "SBO:0000177", "participants": [ { "displayId": "LacI_protein", "name": "LacI_protein", "type": "Protein", "role": "reactant", "SBO": "SBO:0000010" }, { "displayId": "IPTG_LacI_protein", "name": "IPTG_LacI_protein", "type": "Complex", "role": "product", "SBO": "SBO:0000011" }, { "displayId": "IPTG", "name": "IPTG", "type": "SmallMolecule", "role": "reactant", "SBO": "SBO:0000010" } ] }

cjmyers commented 6 years ago

This is looking really good. I've tried it out with SynBioHub, and I got it working. I did though need to check-in one small fix which deals with the participant displayId not being the same as the definitions.

The main issue is the diagram renders each part separately. Ideally, I should be able to just render a ModuleDefinition that includes all the FunctionalComponents and their Interactions in one diagram. The way this would work is that it should not render the Components separately when Interactions are provided, and it should try to render the Interactions all in the same diagram. Clearly, this is not going to work for arbitrary networks yet, so it is fair to have some restrictions on when it will work.

The second issue I had rendering in SynBioHub is that it took a bit to figure out how to create the displayList from SBOL. This is in browser.js for visbol-js. However, this is not the entry point used for SynBioHub. We should add a helper function that takes a ModuleDefinition and returns a displayList. Here is what the code looked like that I craeted:

var component = {
    segments : []
}

sbol.componentDefinitions.forEach(function(componentDefinition) {   
        component.segments = component.segments.concat(
    getDisplayList(componentDefinition, config,
               req.url.toString().endsWith('/share')).components[0].segments[0])
})
var interactions = getInteractionList(moduleDefinition)

locals.meta.displayList = {
        version: 1,
        components: [
            component
    ],
        interactions: interactions
}
asadeg02 commented 6 years ago

@cjmyers Thanks so much for your feedback. For the first issue mentioned above, I wasn't sure what is the expected rendering when sbol files include module definitions so I came up with one which is flexible and at the same is not deviating that much from the current rendering structure. If the expected and ideal rendering is not to render circuits in separate segments when they participate in interactions this is something that I can easily make the current code to take care of. For displaying all interactions happening in the same circuit in one single segment (diagram), I can enhance the current code to do that but it might affect the performance since it requires some sort of preprocessing on interactions list to find all the interactions happening in the same circuit (or lets's say find all the interactions for each circuit), practically it shouldn't be an issue though. for this to happen, i still need to put some restrictions on this approach. for example if a complex which is being degraded is participating in an interaction, I can't display both degradation and non-covelant-binding in one diagram since depending on the offset of DNA part in the circuit participating in the interaction with this complex the layout could be different and coming up with a solution to handle all the cases is kind of adding unnecessary complications. For the problem with SynBioHub displayList, I don't have any information about SynBioHub architecture and visBOL entry point for it. My purpose was to follow the current code structure and reuse the current code as much as I can. I think creating interactionsList in browser.js for visBOL should be correct since the same thing is happening for component definitions in browser.js but if putting that block of code in getdisplayList resolves the issue I can pass interactions to getdisplayList function and create interactionList there and attach it as another key to displaylist object. Another thing I wanted to mention is that my internship period at SAIL has ended at this point (though i will be back again in Sep) but since I like to see my code integrated into visBOL code-base I'm happy to make your requested changes. I think getting rid of redundant circuit segments for those circuits participating in interactions is something I can easily do but for displaying all the interactions in one diagram (this is also doable) but I think maybe I can do that in another pull request.

cjmyers commented 6 years ago

@asadeg02 thanks for all your great work on this. It is a big improvement. If you can at least not show the Components when there are Interactions, that would be a good improvement, since the visualizations tend to be really tall and redundant.

If you have time to help with the merging of interactions into one diagram, that would be great. Alternatively, if you can point me to where the code is for this, that would help a lot too. Assumptions on what can be done is fine as long as they are clear. Would be great if the interaction code sent an error message when it cannot render well, so we can resort back to a table for these cases.

As for the SynBioHub interface, I think I can manage that one.

Finally, a bigger issue is going to need to be addressing the interaction location problem in the SBOL. This will need to be addressed with a more precise (though also more complex) SBOL representation.

asadeg02 commented 6 years ago

@cjmyers Thanks. I have pushed the code for removing those segments participating in interactions from components. Now we only render them with their interactions but i kept the segments having single glyphs in them since I thought they might still be useful. Another thing to note is that all the segments are still in displaylist they just don't get rendered. I can update the displaylist (please let me know if that's expected as well) but since I have to put that block of code in browser.js, I guess i displayList wont get updated for SynBioHub anyway. For merging diagrams, I'm more than happy to help but I just need to let you know the next two weeks are super busy for me. I can also let you know how it should work. the first thing to do in order to merge diagrams is to update or create a new interaction object out the ones which already exist. currently we iterates through the interactions list for rendering interactions but for merging them we first need to know which interactions share the same circuits and crate an object which can get rendered easily using the code (render segment, renderInteraction) that already exists. things like that.... we can talk about it more if necessary. please try it out and let me know if that's what you meant by removing redundancy and maybe we can close this pull request and create a new issue for merging diagrams.

jamesamcl commented 6 years ago

This is excellent, nice work. I'll merge this PR as it's certainly an improvement over not displaying interactions at all. Looking forward to seeing the combined version in the future.