thechiselgroup / biomixer

BioMixer
http://bio-mixer.appspot.com/
16 stars 13 forks source link

Hanging on Specific Expansion #444

Closed everbeek closed 9 years ago

everbeek commented 9 years ago

In the link below, expanding "mesoderm derived structure" adds many nodes, but also hangs the application. http://127.0.0.1:8888/conceptPathToRoot.html?gwt.codesvr=127.0.0.1:9997&initial_vis=term_neighborhood&ontology_acronym=UBERON&full_concept_id=http%3A%2F%2Fpurl.obolibrary.org%2Fobo%2FUBERON_0006591&restURLPrefix=data.bioontology.org

everbeek commented 9 years ago

Debugging is not helping. Moving to log statements...

I might reinstitute the expansion cap that existed in the older version. I should have it anyway...but should I fully diagnose this problem or assume it is because the expansion is getting upwards of 80 nodes?

everbeek commented 9 years ago

I will use a cap and give the user prompts if they want more nodes. Grab 20 at first, then offer 10 more, 10 more, etc. I will look into creating a running estimate of possible final node count, which would be updated as certain REST calls respond.

I might be able to do it by associating the counts with the owning expansion set, and associate that somehow with the fetcher, which would offer the prompt and stonewall further calls when the user no longer desires more nodes.

I am marking callbacks with a type, so that the fetcher can allow link oriented REST calls through always, but halt node ones when the cap is hit. I need to check all places where parseNode() or expandAndParseNodeIfNeeded() are called. I will probably also force callbacks to offer an estimate of the number of nodes they expect to create in their immediate response (that is, give a count of ids they are fetching, if possible).

everbeek commented 9 years ago

I have a problem...I am more concerned with absolute number of objects on screen, not so much how many come in at once or how many have come through (given we can delete things). This means the best place to cap things is in parseNode() or expandAndParseNodeIfNeeded(). I have a concern. If I stop things there, I feel like it would interrupt later attempts to get at the nodes that were jilted. Trying to figure out if my suspicion is supported...

Ok. Suppose the parent call leads to too many nodes. So some of the parent data elements are not parsed to nodes. That data is only cached as the response to that specific URL request. If the jilted parents are requested via a different node expansion, we are fine. But...can I re-expand the same one? I will check that path of logic...

everbeek commented 9 years ago

I need a modal dialog, and was looking at rolling my own, vs Bootstrap, vs Reveal vs SImpleModal...Bootstrap is nicest, but comes with a larger file size, and has various complications with regards to IE, and actually other browsers too (http://getbootstrap.com/browser-bugs/). If I needed Bootstrap for more things, I might be willing to take that risk for the greater number of UI uses. As it is, my menu using only JQuery behaves well, and the modal won't be complicated anyway. Not worth using yet.

Going with roll-my-own, though I may steal this guy's code: http://www.jacklmoore.com/notes/jquery-modal-tutorial/

No wait. I will try SimpleModal first, then PGWModal if that isn't right, then roll my own.

everbeek commented 9 years ago

Had to do a side investigation into an inordinate amount of loops that were being hit. It turns out they are necessary, resulting from the nearly 7000 relations defined to the concept "artery". None of those are expanded, but all are parsed. I am going to look at the REST calls necessary to get all that 99.999% unused information...

For the five loaded concepts, I had relation counts of: artery: 133, ligamentum arteriosum: 3, medial umbilical ligament: 1, mesoderm-derived structure: 931, transformed artery: 6753, vestigial embryonic structure: 15

That's incredible. Also, this is pretty clearly important to note in the Expand Concept and Expand Mappings menu items, as the counts come in #447.

everbeek commented 9 years ago

Back to modal...

everbeek commented 9 years ago

When the relation property node "artery" is added to the graph, I get a stack overflow, possibly infinite loop. I need to fix this before I can get the modal dialog working. ConceptGraph.expandRelatedConcept.fetchCall ConceptGraph.js:593ConceptGraph.checkForNodeCap ConceptGraph.js:458ConceptGraph.expandRelatedConcept ConceptGraph.js:595(anonymous function) ConceptGraph.js:1286n.extend.each jquery.min.js:2(anonymous function) ConceptGraph.js:1284n.extend.each jquery.min.js:2ConceptCompositionRelationsCallback.callback ConceptGraph.js:1249RetryingJsonFetcher.fetch FetchFromApi.js:525ConceptGraph.fetchCompositionRelations ConceptGraph.js:949ConceptGraph.fetchConceptRelations ConceptGraph.js:919FetchOneConceptCallback.callback ConceptGraph.js:1181RetryingJsonFetcher.fetch FetchFromApi.js:525ConceptGraph.expandRelatedConcept.fetchCall ConceptGraph.js:593ConceptGraph.checkForNodeCap ConceptGraph.js:458ConceptGraph.expandRelatedConcept ConceptGraph.js:595(anonymous function) ConceptGraph.js:1286n.extend.each jquery.min.js:2(anonymous function) ConceptGraph.js:1284 ConceptGraph.expandRelatedConcept ConceptGraph.js:595(anonymous function) ConceptGraph.js:1286n.extend.each jquery.min.js:2(anonymous function) ConceptGraph.js:1284n.extend.each jquery.min.js:2ConceptCompositionRelationsCallback.callback ConceptGraph.js:1249RetryingJsonFetcher.fetch FetchFromApi.js:525ConceptGraph.fetchCompositionRelations ConceptGraph.js:949ConceptGraph.fetchConceptRelations ConceptGraph.js:919FetchOneConceptCallback.callback ConceptGraph.js:1181RetryingJsonFetcher.fetch FetchFromApi.js:525ConceptGraph.expandRelatedConcept.fetchCall ConceptGraph.js:593ConceptGraph.checkForNodeCap ConceptGraph.js:458ConceptGraph.expandRelatedConcept ConceptGraph.js:595(anonymous function) ConceptGraph.js:1286n.extend.each jquery.min.js:2(anonymous function) ConceptGraph.js:1284

everbeek commented 9 years ago

There is no canonical way to identify properties that are relations when we get the ontology properties (http://data.bioontology.org/ontologies/UBERON/properties?apikey=efcfb6e1-bcf8-4a5d-a46a-3ae8867241a1), but when we are looking at the concepts and find those property ids, we can see if the value is itself another id (and thus likely another concept), and leave out those that are not (and therefore not possibly another concept).

everbeek commented 9 years ago

Ok, back to modal again for real...

everbeek commented 9 years ago

Trimmed down number of calls to the expansion-checking-function by guarding prior to creating the callback. Before, the guard was right inside the subsequent function, so it didn't matter. Perhaps this breaks encapsulation, but it pays off, because I cut it down from about 280 calls to 5. There were that many that were not supposed to be loaded for a particular expansion. They were relations of the highly linked 'artery'.

everbeek commented 9 years ago

Have the cap dialog logic working with the pre-cap nodes, the user allowed nodes, and with cancelling expansion (no more nodes). When it is cancelled, the user can still re-trigger the expansion via the node menu, but when the user allows a capped number of nodes in, rather than cancelling, the menu item becomes inaccessible regardless of the number allowed in. Fixing that logic.

everbeek commented 9 years ago

I now need to know if an expansion's node fetches have failed with a 404 or other error. Jsonp does not support any errors, by its very nature. Some people use timeouts to catch 404s, but that is a balancing act between picking a timeout duration that will allow all non-error fetches to return, while not making the user wait longer for the fetches to all be resolved.

Using CORS allows for errors. I believe it is time to look into that. I will commit what I have, but the Expand menus will report that there are more nodes to expand if any of them have errored out (commonly a 404).

everbeek commented 9 years ago

I guarded against adding unnecessary wrapped callbacks by adding a check for the node in the graph before each checkForNodeCap() call. I made it underneath such that if this check is not done, it will be ok, but saving on that might matter at some point.

everbeek commented 9 years ago

In Chrome (but not Firefox), hover over is broken, only triggering when I click nodes, which also initiates dragging, but notably does not allow me to drag the node. I checked the previous commit, and it works fine there. I have reviewed the changes in my recent unpushed commit, and cannot see what I did to affect this behavior. Trying to fix this...ok, of course, rebooting chrome did the trick. Onward!

The mapping menu item has no numbers, and does not show as completed when clicked. This is probably not due to outstanding 404s on mapped concepts, as discussed in the previous comment.

Undo/redo don't seem to be registering the expansions properly, with some undo steps not really affecting the graph. This doesn't seem to be super replicable, so I will not try to fix it just yet.

After fixing all these, work on the CORS for 404 handling, so we can get accurate node counts for expansions.

everbeek commented 9 years ago

The node counts for the expansion menus and cap dialog are wrong...they are including both mapping and concept expansions together, I believe. After doing concept expansion, the mapping expansion kept leading to concept links rather than adding mapped concepts. I thought I changed it so that a callback queue was cleared after processing...fixing...

After this, work on the CORS for 404 handling, so we can get accurate node counts for expansions.

everbeek commented 9 years ago

I fixed the mapping issue; I was checking for node caps above the callback creation, rather than before the node creation, so all the fetches went out at once, passing the graph population test, and their nodes were added far after. Sorted that out, and found that there were no numbers in the mapping expansion text because I was using an incorrect variable. Typescript cannot prevent me from doing that entirely, it seems.

It turns out that the 404s I am getting in this case are not among the expanded nodes. I don't want to search for a reliable 404 for this scenario, so I am leaving it be.