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

[Issue#9] - internal/graph: partition algorithm #16

Draft
wants to merge 38 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
e967317
internal/graph: work in progress on partition algorithm
mccurdyc Oct 15, 2019
f75e31b
[WIP] internal/graph: working on recursiveBFS and the calculation of …
mccurdyc Oct 16, 2019
037f784
[WIP] internal/graph: working on calculating edge distances
mccurdyc Oct 17, 2019
a869368
[WIP] internal/graph: graph traversal is done. working on calculating…
mccurdyc Oct 18, 2019
01264a6
internal/graph: first full draft complete. thinking about summation (…
mccurdyc Oct 21, 2019
7b79d8c
internal/graph: moving parition code to its own file and moving metho…
mccurdyc Oct 22, 2019
f783132
splitfile: update to use the new Partition API
mccurdyc Oct 22, 2019
5bc520d
internal/graph: update partition identifier function to check epsilon…
mccurdyc Oct 23, 2019
afd6b38
[WIP] internal/graph: update graph tests to work with new API
mccurdyc Oct 23, 2019
1ba49f1
splitfile: update epsilon value 1 -> 0.1
mccurdyc Oct 23, 2019
2c24453
testdata/src/abc: update testdata to use example in the README
mccurdyc Oct 23, 2019
4f11189
Merge branch 'master' of github.com:mccurdyc/splitfile into mccurdyc/…
mccurdyc Oct 25, 2019
bfd037f
ider: id function uses type instead of position and add tests
mccurdyc Nov 29, 2019
d0aaee7
checkSignature should not return the receiver variable as a related node
mccurdyc Nov 30, 2019
e83341a
checkSignature should not return the receiver variable as a related node
mccurdyc Nov 30, 2019
cb8463e
internal/graph: add methods for returning Roots and Edges of a graph
mccurdyc Dec 3, 2019
57eef69
internal/graph: add source and destination to edges
mccurdyc Dec 3, 2019
b019b87
internal/graph: partition should use new Roots and Edges methods
mccurdyc Dec 3, 2019
ac55b8d
analysis/test: uncomment test from filesystem
mccurdyc Dec 3, 2019
776af2d
add gitignore to ignore testdata/pkg
mccurdyc Dec 3, 2019
37876b2
internal/graph: add placeholder for partition tests
mccurdyc Dec 3, 2019
f75ebe4
splitfile tests
mccurdyc Dec 3, 2019
b5a786a
WIP; working on graph traversal. not working properly
mccurdyc Dec 3, 2019
1007883
WIP; fixed method related. there is still an issue where some edges a…
mccurdyc Dec 4, 2019
1b51055
fix race condition between method node being added first and method b…
mccurdyc Dec 5, 2019
d9dc68c
recursiveBFS: needed to remove the element from the queue in order to…
mccurdyc Dec 6, 2019
6d19fec
add debug logging to give additional information about the identified…
mccurdyc Dec 9, 2019
d32bf9c
set and use defaultWeight of -1
mccurdyc Dec 10, 2019
786e562
WIP; check for default values to avoid setting weights to defaults (l…
mccurdyc Dec 10, 2019
f6aae20
WIP; fix weights. lower inidicates closer
mccurdyc Dec 10, 2019
0333162
dont add default values to minpathweights
mccurdyc Dec 11, 2019
448de64
WIP; refactor recursiveBFS; builds, but havent verified correctness
mccurdyc Dec 11, 2019
80418b2
fix reference bug where edges could be referencing different values f…
mccurdyc Dec 12, 2019
4f34901
internal/graph: simplify recursiveBFS function
mccurdyc Dec 16, 2019
b49641d
internal/graph/partition: simplified bfs function
mccurdyc Dec 17, 2019
f24be6a
internal/graph/node: use -1.0 as defaultWeight
mccurdyc Dec 17, 2019
3ae6e82
internal/graph/partition: update bfs comment
mccurdyc Dec 17, 2019
f1e2a9c
WIP; need to write unit tests...enough of this manual checking
mccurdyc Dec 18, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
testdata/pkg/*
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
github.com/google/go-cmp v0.3.1
github.com/pkg/errors v0.8.1
github.com/stretchr/testify v1.3.0
golang.org/x/tools v0.0.0-20190826060629-95c3470cfb70
golang.org/x/tools v0.0.0-20190918214516-5a1a30219888
golang.org/x/tools/gopls v0.1.7 // indirect
honnef.co/go/tools v0.0.1-2019.2.3
)
7 changes: 7 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ golang.org/x/mod v0.0.0-20190513183733-4bf6d317e70e/go.mod h1:mXi4GBBbnImb6dmsKG
golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
Expand All @@ -34,6 +35,12 @@ golang.org/x/tools v0.0.0-20190425150028-36563e24a262/go.mod h1:RgjU9mgBXZiqYHBn
golang.org/x/tools v0.0.0-20190621195816-6e04913cbbac/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
golang.org/x/tools v0.0.0-20190826060629-95c3470cfb70 h1:Cde4MUY6o3RgxpWu8/7xCjEW7SE22me73ix+NIXtV7s=
golang.org/x/tools v0.0.0-20190826060629-95c3470cfb70/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.0.0-20190918214516-5a1a30219888 h1:ER45Jz0UDQ3e6em1lwXVwuPf96lvyQogb7m+gEbsoPg=
golang.org/x/tools v0.0.0-20190918214516-5a1a30219888/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.0.0-20191014205221-18e3458ac98b h1:EsQHTYgcM562dq02r6y2Yt9VpvvLNIyNECx96XQeolA=
golang.org/x/tools/gopls v0.1.7 h1:YwKf8t9h69++qCtVmc2q6fVuetFXmmu9LKoPMYLZid4=
golang.org/x/tools/gopls v0.1.7/go.mod h1:PE3vTwT0ejw3a2L2fFgSJkxlEbA8Slbk+Lsy9hTmbG8=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
Expand Down
9 changes: 5 additions & 4 deletions ider.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,21 @@ package splitfile

import (
"fmt"
"go/token"
"go/types"
)

type Ider interface {
Pkg() *types.Package
Name() string
Pos() token.Pos
Type() types.Type
}

func Id(ider Ider) string {
id := fmt.Sprintf("%s %s", ider.Name(), ider.Type().String())

if ider.Pkg() != nil {
return fmt.Sprintf("%s %s %d", ider.Pkg().String(), ider.Name(), ider.Pos())
id = fmt.Sprintf("%s %s", ider.Pkg().String(), id)
}

return fmt.Sprintf("%s %d", ider.Name(), ider.Pos())
return id
}
32 changes: 24 additions & 8 deletions ider_test.go
Original file line number Diff line number Diff line change
@@ -1,43 +1,59 @@
package splitfile

import (
"go/token"
"go/types"
"testing"
)

type mockType struct {
s string
}

func (mt mockType) Underlying() types.Type { return nil }
func (mt mockType) String() string { return mt.s }

type mockObject struct {
pkgFn func() *types.Package
nameFn func() string
posFn func() token.Pos
typeFn func() types.Type
}

func (mo mockObject) Pkg() *types.Package { return mo.pkgFn() }
func (mo mockObject) Name() string { return mo.nameFn() }
func (mo mockObject) Pos() token.Pos { return mo.posFn() }
func (mo mockObject) Type() types.Type { return mo.typeFn() }

func TestId(t *testing.T) {
var tests = []struct {
name string
want string
pkgFn func() *types.Package
nameFn func() string
posFn func() token.Pos
typeFn func() types.Type
}{
{
name: "no package",
want: "name 1",
name: "no_package",
want: "name type",
pkgFn: func() *types.Package { return nil },
nameFn: func() string { return "name" },
posFn: func() token.Pos { return token.Pos(1) },
typeFn: func() types.Type { return mockType{s: "type"} },
},

{
name: "package_exists",
want: "package pkg (\"github.com/mccurdyc/path\") name type",
pkgFn: func() *types.Package {
return types.NewPackage("github.com/mccurdyc/path", "pkg")
},
nameFn: func() string { return "name" },
typeFn: func() types.Type { return mockType{s: "type"} },
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
o := mockObject{
pkgFn: tt.pkgFn,
nameFn: tt.nameFn,
posFn: tt.posFn,
typeFn: tt.typeFn,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the variable on range scope tt in function literal (from scopelint)

}

got := Id(o)
Expand Down
37 changes: 20 additions & 17 deletions internal/graph/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,27 +29,30 @@ func (g Graph) ContainsNode(id string) bool {
return ok
}

// Partition returns a slice of nodes that should be split from a given source graph.
//
// TODO (Issue#8): actually add logic to properly partition
func (g Graph) Partition() []*Node {
res := make([]*Node, 0, len(g))

// TODO: for now, just return every node
for _, v := range g {
res = append(res, v)
// Roots finds the many possible roots in the graph.
// The order of the graph traversal matters because if a parent is added _after_
// a child, then that child node won't have the identified parent node.
func (g Graph) Roots() []*Node {
roots := make([]*Node, 0, len(g))

for _, node := range g {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

order matters here

if len(node.Parents) == 0 {
roots = append(roots, node)
}
}

return res
return roots
}

func (g Graph) shouldPartition() bool {
if len(g) <= 1 {
return false
}
// Edges returns all of the edges in the graph.
func (g Graph) Edges() []WeightedEdge {
edges := make([]WeightedEdge, 0)

// TODO: thought; maybe eventually we store meta data about the graph
// (e.g., types of edges such as method and if there are zero methods, maybe we consider not partitioning)
for _, node := range g {
for _, edge := range node.Edges {
edges = append(edges, edge)
}
}

return true
return edges
}
49 changes: 46 additions & 3 deletions internal/graph/graph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,25 @@ func TestNew(t *testing.T) {
}

var (
parentNode = Node{ID: "parent", Edges: make(map[string]WeightedEdge), Parents: make(map[string]WeightedEdge)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parentNode is a global variable (from gochecknoglobals)


parentEdge = map[string]WeightedEdge{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parentEdge is a global variable (from gochecknoglobals)

"parent": WeightedEdge{
Weight: 1.0,
Dest: &parentNode,
},
}

edgeB = map[string]WeightedEdge{
"b": WeightedEdge{
Weight: 1.0,
Dest: &Node{ID: "b", Edges: make(map[string]WeightedEdge)},
Dest: &Node{ID: "b", Edges: make(map[string]WeightedEdge), Parents: make(map[string]WeightedEdge)},
},
}

nodeAWithSingleEdgeB = Node{ID: "a", Edges: edgeB}
nodeCWithSingleEdgeB = Node{ID: "c", Edges: edgeB}
nodeAWithSingleEdgeB = Node{ID: "a", Edges: edgeB, Parents: make(map[string]WeightedEdge)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nodeAWithSingleEdgeB is a global variable (from gochecknoglobals)

nodeCWithSingleEdgeB = Node{ID: "c", Edges: edgeB, Parents: make(map[string]WeightedEdge)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nodeCWithSingleEdgeB is a global variable (from gochecknoglobals)

nodeCWithEdgeBAndParent = Node{ID: "c", Edges: edgeB, Parents: parentEdge}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nodeCWithEdgeBAndParent is a global variable (from gochecknoglobals)

)

func TestAddNode(t *testing.T) {
Expand Down Expand Up @@ -153,3 +163,36 @@ func TestContainsNode(t *testing.T) {
})
}
}

func TestRoots(t *testing.T) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function TestRoots has 27 lines of code (exceeds 25 allowed). Consider refactoring.

tests := []struct {
name string
graph Graph
want []*Node
}{
{
name: "empty-graph",
graph: Graph(make(map[string]*Node)),
want: []*Node{},
},

{
name: "one-root-node",
graph: Graph(map[string]*Node{
"parent": &parentNode,
"c": &nodeCWithEdgeBAndParent,
}),
want: []*Node{&parentNode},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
got := test.graph.Roots()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the variable on range scope test in function literal (from scopelint)


if diff := cmp.Diff(test.want, got); diff != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the variable on range scope test in function literal (from scopelint)

t.Errorf("(%+v) Roots() mismatch (-want +got): \n%s", test.graph, diff)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the variable on range scope test in function literal (from scopelint)

}
})
}
}
43 changes: 35 additions & 8 deletions internal/graph/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,47 @@ import (
"errors"
)

const defaultWeight = -1

// WeightedEdge contains pointers to source and destination Nodes as well as
// the assigned weight of the relationship.
type WeightedEdge struct {
Weight float64
Dest *Node
Weight float64
Source, Dest *Node

ConnectednessStrength float64
MinPathStrengths []float64 // used across multiple graph traversals

Partition bool
Distance float64
}

// Node has an ID and a map of WeightedEdges or weighted relationships to other
// Nodes.
type Node struct {
ID string
Object interface{}
Edges map[string]WeightedEdge
ID string
Object interface{}
Edges map[string]WeightedEdge
Parents map[string]WeightedEdge // TODO: may be able to delete this now

MinPathStrength float64
ShortestPathLen float64 // used for a single graph traversals
}

// NewNode creates a pointer to a new Node with ID, id, and initializes a map of Edges.
func NewNode(id string, v interface{}) *Node {
return &Node{
ID: id,
Object: v,
Edges: make(map[string]WeightedEdge),
ID: id,
Object: v,
Edges: make(map[string]WeightedEdge),
Parents: make(map[string]WeightedEdge),
MinPathStrength: defaultWeight,
ShortestPathLen: defaultWeight,
}
}

// AddEdges adds weighted edges to a source node that signify relationships with other nodes.
// Also adds parents to the destination node.
func (n *Node) AddEdge(dest *Node, w float64) {
// prevent edge between node and itself
if n.ID == dest.ID {
Expand All @@ -37,8 +53,15 @@ func (n *Node) AddEdge(dest *Node, w float64) {

n.Edges[dest.ID] = WeightedEdge{
Weight: w,
Source: n,
Dest: dest,
}

dest.Parents[n.ID] = WeightedEdge{
Weight: w,
Source: dest,
Dest: n,
}
}

// ContainsEdge returns whether or not the graph contains an edge from source to dest.
Expand All @@ -60,5 +83,9 @@ func (n *Node) Valid() (bool, error) {
return false, errors.New("invalid node; edge map must be initialized")
}

if n.Parents == nil {
return false, errors.New("invalid node; parent map must be initialized")
}

return true, nil
}
50 changes: 40 additions & 10 deletions internal/graph/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,26 @@ import (
"testing"
)

var (
nodeA = Node{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nodeA is a global variable (from gochecknoglobals)

ID: "a",
Object: nil,
Edges: map[string]WeightedEdge{},
Parents: map[string]WeightedEdge{},
MinPathStrength: 1.0,
ShortestPathLen: 1.0,
}

nodeB = Node{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nodeB is a global variable (from gochecknoglobals)

ID: "b",
Object: nil,
Edges: map[string]WeightedEdge{},
Parents: map[string]WeightedEdge{},
MinPathStrength: 1.0,
ShortestPathLen: 1.0,
}
)

func TestAddEdge(t *testing.T) {
tests := []struct {
name string
Expand All @@ -15,21 +35,31 @@ func TestAddEdge(t *testing.T) {
}{
{
name: "add-edge-w1.0-to-empty-slice-should-return-edges-with-single-value",
node: &Node{"a", nil, map[string]WeightedEdge{}},
dest: &Node{"b", nil, map[string]WeightedEdge{}},
node: &nodeA,
dest: &nodeB,
weight: 1.0,
want: Node{"a", nil, map[string]WeightedEdge{"b": WeightedEdge{
Weight: 1.0,
Dest: &Node{ID: "b", Object: nil, Edges: map[string]WeightedEdge{}},
}}},
want: Node{
ID: "a",
Object: nil,
Edges: map[string]WeightedEdge{
"b": WeightedEdge{
Weight: 1.0,
Source: &nodeA,
Dest: &nodeB,
},
},
Parents: map[string]WeightedEdge{},
MinPathStrength: 1.0,
ShortestPathLen: 1.0,
},
},

{
name: "add-edge-to-same-node-does-nothing",
node: &Node{"a", nil, map[string]WeightedEdge{}},
dest: &Node{"a", nil, map[string]WeightedEdge{}},
node: &nodeA,
dest: &nodeA,
weight: 1.0,
want: Node{"a", nil, map[string]WeightedEdge{}},
want: nodeA,
},
}

Expand All @@ -38,7 +68,7 @@ func TestAddEdge(t *testing.T) {
tt.node.AddEdge(tt.dest, tt.weight)

if ok := reflect.DeepEqual(*tt.node, tt.want); !ok {
t.Errorf("(%+v) AddEdge(%+v, %f) - mismatch \n%+v", tt.node, tt.dest, tt.weight, tt.want)
t.Errorf("AddEdge() - mismatch \n\twant: %+v\n\tgot:%+v", tt.want, *tt.node)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the variable on range scope tt in function literal (from scopelint)

}
})
}
Expand Down
Loading