-
Notifications
You must be signed in to change notification settings - Fork 4
(Large) Provenance MVP #630
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…feature/network-graph
…feature/network-graph
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This came up in my testing quite a bit since some provenance actions were causing this (they shouldn't and I fixed each instance of it happening as it they came but they were and an unseemly default error page was appearing) so this is just a simple error page I added to replace the default from react-router that we can improve on later should we want to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This hook is very much changed. This previously was checking the remote server itself then returning a callback. Instead, all this does is return a callback and the check for the remote server now happens in interaction/logics to avoid this check happening more than once per application load. Previously it was happening several times since the component this hook lived in was tied to what created the context menu for file interactions which gets destroyed in certain scenarios. This got updated in this changeset because the sheer amount of logs this was generating was very distracting
|
How would this look if each zarr had 10 segmented cell images? Seems like it would be hard to fit/display that. That is more a UX question that a code one, though. |
This would depend on the organization strategy chosen. The default is |
| * on a previous run used up all the node search afforded | ||
| */ | ||
| public get hasMoreToSearch(): boolean { | ||
| return this.numberOfNodesAfforded <= 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this right? we are saying there are more nodes to search if == 0 (or less but I dont think thats possible)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return this.numberOfNodesAfforded > 0;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yea this is a bit confusing naming wise (open to suggestions) this is answering the question of whether or not there could be more nodes to search not if we can search more nodes during this run. The idea here is that if we exhaust our searching without running out of afforded nodes then we can assume if have searched everything we possibly can
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think theres two things that are confusing here. 1) the name of the function hasMoreToSearch doesnt refrence what its searching and 2) numberOfNodesAfforded seems like it should be a constant from the name. Maybe another value like this.remainingNodeAllocation combined with hasCompletedNodeSearch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed hasMoreToSearch to hasMoreNodesToSearch - does that help for point 1? For point 2 I'm unsure of a better name - we need it to be a state variable as opposed to a constant because it tracks how many nodes we have checked that are novel
Context
We want to provide users the ability to show context about how files relate to each other AKA "provenance" this includes things like how a file was generated (ex. script) or more conceptual things how a file relates to a publication.
(Shoutout @aswallace started this implementation :) )
Changes
Lots of changes! New components for generating the provenance graph were created, though much of the rendering is managed by
xyflow(seecomponents/NetworkGraph). Logical entities were also created to hold complexity (seeentity/Graph). Those are the major changes you should check out. However, additional cleanup to some busy components was done to alleviate some duplicate work that needed to happen in the graph components & to make testing easier.Testing
Lots of manual input of provenance & new unit tests. The lower level components like
FileNodeandMetadataNodecan't be tested using our current testing system unfortunately - hopefully this can be remedied in the upcoming improvements to our infrastructure.Try it here using a small, but high fidelity provenance dataset. Working to get a provenance table going for using with the AICS FMS source directly
Example using the above linked dataset:

This was created using this provenance source:

& this dataset:
