Skip to content
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

Reduce dictionary lookups and Maybes in the Graph API #11

Closed
ggazzi opened this issue Mar 26, 2017 · 4 comments
Closed

Reduce dictionary lookups and Maybes in the Graph API #11

ggazzi opened this issue Mar 26, 2017 · 4 comments
Assignees

Comments

@ggazzi
Copy link
Member

ggazzi commented Mar 26, 2017

The API of module Graph.Graph forces frequent lookups into the association lists, which probably incurs a performance penalty. Also, its functions often return Maybes where the result could be guaranteed to exist.

In order to reduce these issues, a notion of "node context" could be introduced, which allows the user to access the incident edges of a node. This would make the lookups implicit, and the well-formedness of the graphs ensures that such lookups do not fail.

The following is a sketch of the additions/changes to Graph.Graph module

data Node n =
    Node NodeId (Maybe n)

data Edge e =
    Edge EdgeId NodeId NodeId (Maybe e)

type NodeInContext n e =
    (Node n, NodeContext n e)

type EdgeInContext n e =
    (NodeInContext n e, Edge e, NodeInContext n e)

-- The constructor for NodeContext is not exported.
data NodeContext n e =
    NodeContext NodeId (Graph n e)

lookupNodeInContext :: NodeId -> Graph n e -> Maybe (NodeContext n e)

incidentEdges :: NodeContext -> [EdgeInContext n e]

incomingEdges :: NodeContext -> [EdgeInContext n e]

outgoingEdges :: NodeContext -> [EdgeInContext n e]

As an example of usage, this is an implementation of depth-first search using the contexts.

depthFirstSearch :: NodeInContext n e -> [Node n]
depthFirstSearch initialNode =
    search IntSet.empty [initialNode]
    
  where
    search _ [] = []
    
    search visitedNodes ((node, context) : rest) =
      if node `IntSet.member` visitedNodes then
        search visitedNodes rest
      else 
        let
          nextNodes =
            [ target | (_, _, target) <- outgoingEdges context ]
              
        in
          node :: search (IntSet.insert (nodeId node) visitedNodes) (nextNodes ++ rest)
@ggazzi ggazzi self-assigned this Mar 27, 2017
@ggazzi ggazzi added this to Internal Quality in verigraph Mar 27, 2017
ggazzi pushed a commit that referenced this issue Mar 30, 2017
As discussed in issue #11
@ggazzi
Copy link
Member Author

ggazzi commented Mar 30, 2017

Now that the alternatives were added, should we deprecate the functions nodesOf, sourceOf, sourceOfUnsafe, targetOf, targetOfUnsafe and getIncidentEdges, which perform explicit lookups?

@jsbezerra
Copy link
Member

I think it is the best approach, but gradually and at a new branch.

@lm-rodrigues
Copy link
Member

I think that @jsbezerra's strategy is a good choice 😄

ggazzi pushed a commit that referenced this issue Mar 30, 2017
@ggazzi ggazzi removed the question label Mar 30, 2017
@ggazzi ggazzi closed this as completed Mar 30, 2017
@ggazzi
Copy link
Member Author

ggazzi commented Mar 30, 2017

Solved and merged into master.

@ggazzi ggazzi removed this from Internal Quality in verigraph Mar 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants