search

MICommunity / ComplexViewer

web based visualisation of molecular interaction data
http://complexviewer.org
Apache License 2.0
12 stars 5 forks source link

CPX-7083 broken in viewer #202

Open colin-combe opened 4 months ago

colin-combe commented 4 months ago

where there's multiple references to same complex, that complex needs to be cloned (once per reference).

https://github.com/MICommunity/ComplexViewer/issues/199#issuecomment-2205340761

in this case, it's not working where there are nested subcomplexes.

(@EliotRagueneau - could be a bit of time before i have this fixed)

EliotRagueneau commented 4 months ago

Do you think we could be of any help?

colin-combe commented 4 months ago

what's at issue is the code in https://github.com/colin-combe/ComplexViewer/blob/master/src/js/clone-complex-refs.js

i think it gets complicated because we also have to update the linked features in the correct way, and then for CPX-7083 we need to deal with nested subcomplexes.

I think the code that's there is quite hard to understand... i've been fiddling with it and so far it seems only exactly what's there works for the the other cases we have. (CPX-5684, CPX-6230, CPX-5605, CPX-5897).

What's there isn't a correct solution but just happens to work for the other cases, i think.

To be honest, it's kind of hard to figure out exactly what you need to do regardless of writing the code.

Feel free to look at the code or suggest pseudo code about how to solve the problem.

One thing that makes the code confusing is the distinction between complex as a participant and complex as an interaction.

I notice that when it is a participant i don't have a good way of checking if it's a complex and am searching the id for "complex"? https://github.com/colin-combe/ComplexViewer/blob/master/src/js/clone-complex-refs.js#L15

You understand the more general problem about the need to clone the complexes?

bmeldal commented 4 months ago

The other issue is that CPX-7083 is made up of 2x CPX-7041. But see "remark-internal" = "Actual stoichiometry should be 1:1 but it's currently blocked by the duplicate detector as it doesn't look at sub-sub-complexes as participants. Hence why stoichiometry is set to 0 for now." and "caution" = "This complex is almost duplicate (Only Stoichiometry is different) with complex acs:: EBI-25491938" (although that complex is just nsp9 dimer - that ID must be wrong and I guess it should be CPX-7041 - the "monomer" of this supercomplex)

I'm sure that's messing with the expansion, too. We were working on the duplicate detector when I was leaving. Anjali was the only one left to work on it at he time and didn't have the time to finish it all either. I think it just got left in an "almost workable" state. It was never going to work for all situations.

bmeldal commented 4 months ago

I'm free pretty much all day tomorrow if you want to have a chat and pick my brain about the duplication detector.

EliotRagueneau commented 4 months ago

Thanks a lot @bmeldal , I think that could really be the source of the problem, you have a brilliant memory! To explain fully, this duplicate checker doesn't work recursively, so it only checks for subcomplex particpants, but not if they are complexes themselves. By introducing recursiveness to the function, the editor will be able to understand the difference between CPX-7083 and CPX-5687, which will allow us to put real stoichiometry number for CPX-7041. Hopefully, that will help the viewer to render things correctly.

This probably mean that you should wait @colin-combe before trying to fix that one, because the file will change soon normally

bmeldal commented 4 months ago

Yes, @EliotRagueneau you put it into proper language :)

I spent long enough on this set of complexes - didn't really have much else to do in the lockdowns either ;-)

They were fun and challenging to curate - and pushed our model to it's extremes :)

EliotRagueneau commented 4 months ago

Hi @colin-combe and @bmeldal . We have managed to fix the editor to support the correct stoichiometry. The new JSON doesn't fix the problem though unfortunately, as it doesn't impact features, so this complex is still problematic.

Here's the new version of the JSON CPX-7083.json

As previously stated, I will deploy the version 2.2.2, but this is still a pending issue then.

colin-combe commented 4 months ago

this is still a pending issue then.

Yes, i expected that - https://github.com/MICommunity/ComplexViewer/pull/201#issuecomment-2208586064