This commit modifies the Graph class so that it permits dangling edges;
that is to say, edges whose src or dst are not present in the graph.
Dangling edges may be directly added to the graph, or existing edges may
become dangling if their src or dst is removed.
This change is prerequisite to #1136; if we require that nodes have
metadata, we should also make it possible to add edges to nodes that
don't yet exist, as the plugin creating an edge may not have access to
the full metadata needed to add the node.
To support this change, there is now an `isDanglingEdge` method on the
graph, which reports whether or not the edge is dangling. Also,
`Graph.edges` requires that the client make an explicit choice on
whether dangling edges are desired. This ensures that we do not
accidentally include dangling edges in a case where they are
inappropriate (e.g. creating a Markov chain) or accidentally discard
dangling edges when they are needed (e.g. when merging or serializing).
The Graph's invariant checker has been updated to reflect the new
semantics.
The Graph compat version has been bumped, since this is a break in
backwards compatibility.
Note that this commit does not change the behavior of any plugins; that
is to say, no plugins create dangling edges (yet).
Test plan: The advanced graph test case has been updated to include
dangling edges. The tests for Graph, PagerankGraph, and
GraphToMarkovChain have been updated. `yarn test --full` passes.
This commit modifies the base `Graph` class so that nodes are now
represented by `Node` objects rather than `NodeAddressT`. The intention
is to start adding additional fields (e.g. description and timestamp) to
nodes, although that is not included in this commit.
See #1136 for rationale.
Test plan: The graph is very well tested, and this commit adds
additional tests and invariant checking. Some additional test code
needed update. `yarn test --full` passes, and the SourceCred UI works as
expected.
PagerankGraph's `node` and `edge` getters returned null for unavailable
entries, rather than undefined. This is inconsistent with general JS
behavior, and with the base Graph. I've now cleaned it up.
Test plan: unit tests updated; `yarn test` passes.
Every GitHub entity now has a `description` method which returns a short
markdown description. These will be added to the graph as part of #1136.
Test plan: Inspected snapshots, `yarn test --full`.
This will allow timeline cred (#862) to do a better job of flowing cred
across reaction edges. (Very old reactions should not be moving a lot of
present-day cred.)
Test plan: Inspected snapshot changes.
Every GitHub entity from `RelationalView` now has a `timestampMs`
method. This replaces the standalone `createdAt` method.
Test plan: Snapshots look good.
It's an extension of #1152 induced by #1175.
It's a very simple change; I just changed the schema, and
`scripts/update_snapshots.sh` took care of the rest.
Test plan: Inspected snapshots and generated flow types.
This pulls distribution related code out of `markovChain.js` into the new
`distribution.js` module, and from `graphToMarkovChain.js` into
`nodeDistribution.js`.
Since the `computeDelta` method is now exported, I've added some unit
tests.
Test plan: `yarn test` passes.
As #1136 will be moving timestamps into the graph, we no longer need
`createdAt` method in the `AnalysisAdapter`. Actually, we no longer need
the adapter/loader distinction introduced in #1157. I haven't taken the
time to remove the `BackendAdapterLoader` concept because a) we may need
it later, and b) if we don't, I'll likely remove the `AnalysisAdapter`
concept entirely, in favor of having plugins directly save `graph.json`
files to a known location.
Test plan: `yarn test` passes.
I added `mentionsAuthorReference` based on an untested hypothesis that
they would be useful. With the passage of time, I've never seen any
evidence that they actually improve cred socres (their impact seems
negligible), and they add complexity.
In the future, "go-fishing" style heuristics like this should not merge
unless they are of clearly demonstrated value. Also, it would be better
to add stuff like this via a standalone plugin rather than in the core
GitHub logic.
Undoes #806.
Test plan: `yarn test`
Now that the graph is saved by default as a part of load, users who need
the graph can grab it directly from the `$SOURCECRED_DIRECTORY`. If we
really need a command line util for grabbing it, we should rewrite that
command to just grab the graph from that spot rather than re-computing
it.
Test plan: `yarn test`
As of the timeline cred work, I'm shifting emphasis away from raw
PageRank results, in favor of timeline pagerank results. As such,
there's no need to have load save the regular pagerank results on
creation.
As of #1136, there will be no need for timestampMap, as that data will
be present directly in the graph. As the timeline cred UI will depend on
the full graph for analysis, let's save the graph instead.
Test plan: `yarn test` and snapshot inspection.
This commit adds new helper methods for creating test nodes (`node` and
`partsNode`) and for creating test edges (`edge` and `partsEdge`) to
graphTestUtil.js.
This is very helpful in light of work related to #1136. I'm going to
change the concept of "node" from a raw address to an object, add fields
to that object, and add fields to the `Edge` type. If done naively, we
would need to change all the test code across the project for every one
of those changes.
By centralizing the creation of test nodes and edges behind the new
functions, we can update all the test code in a single place.
This change is trivial from a conceputal perspective, and very
broad-reaching from a code-touching perspective. It should be easy to
review, because if tests pass then the change is probably working as
intended. :)
Test plan: `yarn test`
In #1132 and #1134, I started work on the Odyssey plugin. However,
before getting it to a state where it's usefully included in SourceCred,
I decided to pivot to focus on timeline cred first.
Now I'm merging significant refactors as a part of timeline cred
(#1136). As a side effect of this refactor, the Odyssey plugin should
undergo significant changes (OdysseyInstance is now basically redundant
with base Graph.) Rather than incrementally update unused code, I elect
to remove the plugin. This code should be revived on a side branch, and
then merged into master once we have a fully functioning prototype.
Test plan: `yarn test` passes.
This commit refactors the Graph class so that rather than having
separate maps for inEdges and outEdges, there is a single incidentEdges
map, which contains objects with inEdges and outEdges.
This is motivated by a forthcoming big change as part of #1136; namely, to
allow storing dangling edges in the graph. Once we do so, we'll need a
consistent source of truth that enumerates all of the node addresses
which are accessible in the graph (either because they correspond to a
node in the graph, or because they are the src or dst of a dangling
edge). We could do this by adding another field to graph which tracks
this set, but by making this refactor, we can instead use the key set of
_incidentEdges as the source of truth for which node addresses are
present.
Besides being motivated by #1136, I think it's cleaner in general. Note
there are fewer ways for the graph to be inconsistent, as it's no longer
possible for inEdges and outEdges to have inconsistent sets of node
addresses.
The most complicated piece of this change was updating the automatic
invariant checker. It was no longer possible to test 3.1 and 4.1
separately, so they needed to be merged into a new invariant. Rather
than re-enumerate the invariants, I called the new one the 'Temporary
Invariant', because it is going to disappear in a subsequent commit.
Test plan: `yarn test` passes. Since Graph has extremely thorough
testing, this gives me great confidence in this commit. Note that no
observable behavior has changed.
The generated GitHub GraphQL flow types are a kind of snapshot, and it
can be hard to remember/discover how to update them when making changes
to schema. Therefore, I've included them in the snapshot update script.
Test plan: I used it to update the types, and it worked as intended.
At present, the Git commit node type lives in a strange state of shared
responsibility between GitHub and Git. The Git plugin is nominally
responsible for it, but its render method tries to show a hyperlink to
GitHub -- which is awkward for many reasons, including that the same Git
commit could have multiple hyperlinks on GitHub.
This commit resolves that issue by separating the existing commit type
into two: the Git Commit type, which is owned by the Git plugin and
doesn't have hyperlinks or any fancy GitHub metadata, and the GitHub
Commit, which is owned by the GitHub plugin, corresponds to a unique
database id in GitHub, and has a corresponding GitHub url.
The two commits are connected by a CorrespondsToCommit edge type, which
links from the GitHub commit to the corresponding Git commit.
This is necessary for #1136, as if we want to make descriptions a part
of the graph payload, we need for descriptions to be unique for a given
address--and descriptions are only unique if we identifiy each GitHub
commit pointer as a separate address.
Test plan: The unit testing in this part of the codebase is light, so I
verified that the frontend work as expected for `sourcecred/sourcecred`
and `sourcecred/research`. The new node type and edge type appear
properly in the UI, the GitHub commits are connected to their Git
counterparts, etc.
A long time ago, we made graph views for git and github. These are
interfaces over the graph which allow retrieving nodes' relations, e.g.
finding the parent address of a commit just using the graph.
These are fairly complex, and have seen almost no use at all. The one
thing they are used for is implementing invariant checking. The
invariant checking is nice in principle, but since we only apply it to
the example data, its of very limited value in practice.
Since I'm planning a significant Graph refactor (#1136), I'd rather delete this
code than continue to maintain it, since I think it's complexity/value
ratio is unfavorable.
Test plan: `yarn test`
This is another minor silent test failure: the error message thrown by
loadIndividualPlugin when a GitHub token is not available is not quite
the error that was specified in the test.
There were two issues: that we were testing for the wrong error message,
and that the failure didn't fail the test. I fixed both issues (by
changing the message thrown to match the test, and by having the test
_return_ the expectation that the promise will reject, and by expecting
there to be one assertion.)
Test plan: `yarn unit cli/load` no longer shows any unhandled promise
rejection warnings. If the test is modified so that it checks for the
wrong string, it now properly fails rather than passing with an
unhandled rejection.
Prior to this commit, if you run `yarn unit cli/load`,
you would see a lot of unhandled promise rejection warnings related to
the fact that load calls a `saveTimestamps` function which expects the
SourceCred directory to contain the results of really loading SourceCred
plugins. However, when testing, these functions have been mocked, and so
saveTimestamps rejects. This rejection was not caught, and polluted the
test output without actually failing the tests.
This commit updates the tests so that saveTimestamps can be mocked (via
dependency injection) and we can both verify that it is invoked
correctly, and not pollute the test output with spurious warnings.
Test plan:
- `yarn test --full` passes
- `yarn unit cli/load` produces far fewer
UnhandledPromiseRejectionWarnings (there is still one unrelated one)
- loading sourcecred/research still works (as a canary)
Note: the PR which introduced this issue is #1162.
As discussed in #1163, #1162 caused `sourcecred load` to start failing
for real repos (e.g. sourcecred/research). This commit merges a
somewhat hacky fix.
Test plan: `yarn test` passes, and `sourcecred load sourcecred/research`
works.
Fixes#1156
When users export a GitHub API token that has insufficient privleges
or has been revoked, we have been using a catch all error with retry
to handle it. This change adds a new error type for bad credentials
and does not retry.
Test plan:
There are no unit tests that cover this, however, you can test the
change by supplying a revoked token and attempting to load a GitHub
repo.
This modifies `sourcecred load` so that it saves timestamp information
for all of the loaded plugins in a single aggregated map.
This is quite convenient, as it saves consumers of timestamp information
from needing to worry about the (rather hacky) implementation whereby
the data is fed from each adapter. Instead, consumers can just load the
timestamp map. This will also make it much easier to use timestamp info
in the research codebase.
Test plan: The timestampMap module has testing around generating the map
from the adapter and nodes, writing it, and reading it.
I haven't added any testing to the `load` CLI command. I think it would
be redundant as the updated snapshot test reveals that the map is
getting serialized properly.
Tests pass, and I have inspected the snapshot
This commit modifies `PagerankGraph.runPagerank` so that rather than
always starting from a uniform distribution, it starts with the
PagerankGraph's existing score distribution. The PagerankGraph
initializes with a uniform score over nodes, so it has the exact same
behavior on the first time that runPagerank is called. On subsequent
calls, PageRank will likely converge a lot faster, because it's starting
from converged scores. (It should still be a lot faster in cases where
e.g. the user has tweaked the weights.)
In certain degerate cases, this could change the resultant scores.
Specifically, if there are isolated nodes in the graph and alpha=0, then
the isolated nodes' final scores depends on the initial score. In
general, I think this won't be an issue as we expect alpha > 0 in normal
usage.
Test plan: I added a unit test to verify this property, by checking that
running PageRank with maxIterations==0 on an already-converged graph
results in a still-converged graph. Also, existing tests pass.
I think we can now close#1020.
As SourceCred has evolved, we've grown more and more snapshot tests that
are not included in Jest. The GitHub plugin has two ad-hoc snapshot
tests, the Git plugin has one, and the sharness test suites have one.
This makes it difficult to keep track of where to update snapshots when
core changes are made. To fix this, I've added a script,
`scripts/update_snapshots.sh`, which updates snapshot tests across the
project.
Test plan: I removed existing snapshots across the codebase, ran the
snapshot tester, and they correctly regenerated.
* Refactor Loader from AnalysisAdapter
At present, the only data the AnalysisAdapter provides is the Graph, so
the AnalysisAdapter has a `load` method which directly returns the
graph. I'm planning to add a `createdAt` getter to the adapter as well,
which also will depend on loading the data. To make this change
convenient, I'm starting by refactoring an AdapterLoader out, which
manages loading data from disk, so that once we have an AnalysisAdapter,
it already has all relevant data loaded. Then, it will be much easier to
add a `createdAt` method.
Test plan: Tests updated, flow passes.
* Add `createdAt` to the analysis adapter
A big step forward for time-varying cred. This will make `createdAt`
timestamps available for PageRank analysis.
Test plan: Added new unit tests. Inspect the snapshots. Run `yarn test`.
Thanks to @wchargin for [catching it].
[catching it]: https://github.com/sourcecred/sourcecred/issues/1151#issuecomment-494256526
Generated this change via:
```
$ yarn backend
$ (cd sharness; UPDATE_SNAPSHOT=1 ./test_load_example_github.t -l)
```
Test plan: `yarn test --full`, excluding the known (local-only) failure
described in #1151
Modifies the Git plugin so that we now track commit author dates.
Similar to in #1152, they are encoded in MsSinceEpoch.
Test plan: `yarn test --full` passes, except for the pre-existing
failure discussed in #1151.
Thanks to @s-ben for a conversation which motivated these changes.
Updates github schema to include createdAt timestamps, and then updates
the RelationalView to provide those timestamps as MsSinceEpoch.
I added createdAt timestamps to Repos, Issues, Pulls, Reviews, and
Comments, as these correspond to GitHub graph nodes where I think
time-based filtering is relevant. I didn't add them to Users, Reactions,
or Commits. Reactions, because they correspond to edges not nodes. (We
could consider doing the time filtering on edges too, but I'd rather
keep it simple for now.) Commits, because they're owned by a different
plugin. Users, because... in a certain sense the user identity is
timeless, the time factoring is mostly so we can evaluate how users'
cred varies over time.
Anyway, it will be easy to add more fields later if we need them.
Test plan:
- Inspect snapshot changes
- Ran `yarn test --full`
- Its only failure is pre-existing, per #1151
Thanks to @s-ben for some motivation and discussion about this change.
This adds a WeightsFileManager component that allows the user to save or
load weights in the cred explorer. Clicking the download icon downloads
the weights, clicking the upload icon uploads them.
I also did a slight refactor to the FileUploader so that it no longer
always provides the file upload icon, instead the instantiator passes
children which act as the upload clickable. Seemed more consistent.
Test plan: No tests added, but I manually tested that upload and
download both work.
* Add FileUploader with inspection test
TODO: get it working
* Add a FileUploader component, with inspection test
This adds a FileUploader component, which allows the user to upload JSON
files. Rather than using automated testing, it has an inspection test.
The inspection test may be run by navigating to:
http://localhost:8080/test/FileUploader/
This commit also adds some basic utility functions for defining
inspection tests to `routeData.js`. We should improve support for
inspection tests in the future; see [#1148].
[#1148]: https://github.com/sourcecred/sourcecred/issues/1148
Test plan: Ran the included inspection test.
This commit refactors the `analysis/weights` module so that there's
a single top-level type called `Weights` which includes all of the
user-specified weights for running PageRank. This includes both manually
specified type weights, and per-node weights.
In contrast to the old weights module which had weighted types (i.e. the
type bundled with the weight), this only records the weight choice
(keyed by address). Similarly, the map is empty unless the user has
explicitly chosen a weight. This has a few advantages:
- For planned work on serializing weights, it's convenient that
there's a single unified object that bundles all the weights.
- For planned work on serializing weights, it's convenient that we only
record actual choices, not the default values (defaults are provided
within the WeightsToEdgeEvaluator). This way, we don't need to manually
filter out the default weights for serialization. (We want users who
have not made any choice about the type weights to automatically get new
type weightings we publish in sourcecred/sourcecred).
- Overall, the WeightConfig data piping has become a lot simpler.
While making these changes, I threw away the PluginWeightConfig
component, instead inlining it in the top-level WeightConfig. I also
moved responsibility for the WeightConfig out of the PagerankTable, and
into the containing App; the WeightConfig is created and wired by the
App, and then passed as a whole component into the PagerankTable via
props. This means that PagerankTable does not need to get a bunch of
extra props for piping the state changes into WeightConfig.
I also threw away some frontend test code for the WeightConfig
component. Over the course of the past few months, I've observed that
the value of a lot of the frontend-wiring-testing is pretty low (i.e.
they are not catching issues, because Flow catches them) and the cost of
maintaining the testing is high. I'm now inclined to believe that we
shouldn't be testing routine component wiring via explicit tests,
because it's very easy to verify that the behavior is correct by
interacting with the UI, and the tests are expensive to write and
maintain.
For "core logic" (e.g. behaviors of types and data structures,
algorithms, etc) I continue to believe testing is very valuable. One
heuristic I'll start applying is: "can I achieve high confidence that
this code is correct, without tests, using little effort by manually
inspecting the frontend?". If the answer is yes, I'm unlikely to write
tests.
Test plan: `yarn test` passes, and the UI continues to function,
including changing manual and type level weights and observing cred
changes.
Node and Edge types are increasingly important in SourceCred, as we use
them to decide what weights to provide, what description logic to use,
etc. In #631, we hit a bug around not finding a type matching a
particular node, and in #640 I responded by creating a
"FallbackAdapter", which matches any node, and a
"Static/DynamicAdapterSet", which are abstractions for finding adapters
in a way that guarantees the presence of the fallback adapter.
I think this solution is actually pretty brittle. It only works if we
are disciplined in only ever accessing node and edge types using a
context that included the FallbackAdapter, and it requires us to
centralize all of the "type-not-found" logic in one place (the fallback
declaration) irrespective of what the locally appropriate solution is.
Looking back at #631, the core issue was that we had a getter that
promised to always give a matching type and adapter, rather than
returning a possibly undefined adapter. We fixed this by attempting to
ensure that there always would be a matching adapter. I think it would
be better to have the methods honestly report that the adapter might be
null/undefined, and let the client code handle it appropriately.
This commit implements that change. It's motivated by me being
frustrated at the fallback adapter while doing other refactoring.
Test plan:
Unit tests have been updated and `yarn test` passes. Also, I
experimentally removed the Git plugin from the list of adapters for both
the backend and frontend, and verified that the frontend UI and the
pagerank and export-graph commands still worked.
This refactors the weights module (and downstream consumers) so that
rather than tracking forwardWeight and backwardsWeight as separate
fields, we have an `EdgeWeight` type with a forwards and backwards
property. I feel this makes the code a little cleaner and wanted it a
few times while doing other refactors in this module.
Note that the graphToMarkovChain module has a distinct `EdgeWeight` type
which serves a similar purpose but happens to have different field
names. I've added a TODO to rename those fields for consistency.
Test plan: Since this is slight data structure re-organization, no new
tests are required; that `yarn test` passes is sufficient.
This pull request adds a weight slider to every NodeRow in the explorer,
enabling the user to manually set a weight for that node. The weights are
multiplicative with the type level weights, so that they can be changed
independently (e.g. you can have a comment that is weighted 2x higher than
regular comments, but still have comments get a low weight in general).
This pull coordinates a number of different changes across the codebase, all of
which are tested:
Adding support for manual weights in the weights and
weightsToEdgeEvaluator modules.
Modifying pagerankTable.TableRow so that it can show a slider in the second
column.
Adding piping for manual weights into the PagerankTable shared props, and
into the explorer app
Adding the slider to the NodeRow class that displays the current weight,
and can trigger the upstream weight change
Ensuring that the runPagerank call in the explorer actually uses the manual
weights
At present, there is no way to save these weights (they are ephemeral in the
frontend) and so this is clearly a prototype/tech demo level feature rather
than being ready for real usage. Correspondingly, CLI pagerank command always
uses an empty set of manual weights. I plan to remedy this in a follow-on pull
request.
Test plan: Run the included unit tests (yarn test) and also spin up the UI,
verify that it visually looks good in both Firefox and Chrome, and verify that
changing the weights and then re-running PageRank actually causes the cred of
the modified node to change.
Review plan: In addition to carefully reading the code, ensure that all of the
changes described a few paragraphs up are actually tested.
Merge plan: Squash and merge.
Thanks to @s-ben for proposing this feature in Discord, and to everyone
discussing its implications in this Discourse thread.
In #1140 I rename a field in PagerankGraph.ScoredNode, and in #1141 I
added a new test which referenced the field.
I merged them in quick succession, introducing a conflict. This PR fixes
it.
Test plan:
`yarn test` passes.
This commit adds a `setEdgeEvaluator` method to `PagerankGraph`,
and modifies the constructor to use that method. This will allow us to
use PagerankGraph in the explorer UI, so that we can update edge
weights without fully regenerating the graph.
Test plan: I've added new unit tests that verify basic properties of how
the edge weights are getting set and consumed.
It makes more sense that a ScoredNode has an address than that it has a
sub-node (which is an address).
Test plan: It's a simple field rename; `yarn test` suffices
Resolves#1067
Adds the CLI commands:
`sourcecred clear --all` -- removes the $SOURCECRED_DIRECTORY
`sourcecred clear --cache` -- removes the cache directory
`sourcecred clear --help` -- provides usage info
`sourcecred clear` -- prompts the user to be more specific
Test plan:
The unit tests ensure that the command is properly wired into the
sourcecred CLI, including help text integration. However, just to be
safe, we can start by verifying that calling `sourcecred` without
arguments lists the `clear` command as a valid option, and that
calling `sourcecred help clear` prints help information. (Note: it's
necessary to run `yarn backend` before testing these changes)
The unit tests also ensure that the command removes the proper
directories, so there isn't really a need to manually test it,
although the reviewer may choose to do so to be safe.
Although out of scope for unit tests on this function, we can also do
integration tests, to make sure that running the clear command doesn't
leave the sourcecred directory in an invalid state from the perspective of the `load` command.
```js
$ yarn backend;
$ node bin/sourcecred.js load sourcecred/example-github;
$ node bin/sourcecred.js clear --cache;
$ node bin/sourcecred.js load sourcecred/example-github;
$ node bin/sourcecred.js clear --all;
$ node bin/sourcecred.js load sourcecred/example-github;
```
The expected behavior of the above command block is that the load command never fails or throws an error.
@decentralion and I discussed the scenario where `rimraf` errors.
We decided that testing this scenario wasn't necessary, because
`rimraf` doesn't error if a directory doesn't exist, and
rimraf's maintainer suggests [monkey-patching the fs module]
to get rimraf to error in testing scenarios.
Thanks @decentralion for reviewing and pair-programming this with me.
[monkey-patching the fs module]: https://github.com/isaacs/rimraf/issues/31#issuecomment-29534796
This commit integrates an bare skeleton of the odyssey frontend that we
implemented in the [odyssey-hackathon] repository. You can see the
working frontend that we are trying to port over at
[sourcecred.io/odyssey-hackathon/][scio].
The prototype in the other repository has some tooling choices which are
incompatible/redundant with decisions in our codebase (sass vs
aphrodite), and requires some tools not yet present here
(svg-react-loader). This commit includes the build and integration work
needed to port the prototype frontend into mainline SourceCred. The
frontend scaffold isn't yet integrated with any "real" Odyssey data.
One potential issue: right now, every page that is rendered from the
SourceCred homepage is contained within a [homepage/Page], meaning that
it has full SourceCred website styling, along with the SourceCred
website header. The [application][scio] also has a header. Currently, I
work around this by having the Odyssey UI cover up the base header (via
absolute positioning), which works but is hacky. We can consider more
principled solutions:
- Finding a way to specify routes which aren't contained by
[homepage/Page]; maybe by adding a new top-level route
[here][route-alternative].
- Unify the headers for the Odyssey viewer and the page as a whole
(sounds like inappropriate entanglement?)
- Have a website header and also an application header (sounds ugly?)
[homepage/Page]: ee1d2fb996/src/homepage/Page.js
[route-alternative]: ee1d2fb996/src/homepage/createRoutes.js (L17)
Test plan: Run `yarn start`, and then navigate to
`localhost:8080/odyssey/`. observe that a working website is displayed,
and that the cred logo next to the word "SourceCred" is loaded properly
(i.e. svg-react-loader is integrated properly). Observe that there are
no build/compile errors from either `yarn start` or `yarn build`. Also,
observe that the UI looks passably nice, and that if the number of
elements in the entity lists is larger than can be displayed, the
sidebar pane scrolls independently.
The UI was tested in both Chrome and Firefox.
[odyssey-hackathon]: https://github.com/sourcecred/odyssey-hackathon
[scio]: https://sourcecred.io/odyssey-hackathon/
Thanks to @jmnemo, as the implementation is based on [his work].
[his work]: https://github.com/jmnemo/hackathon-event/
This commit puts in a basic data model for the Odyssey plugin. It's
built around the `OdysseyInstance` class, which is basically a Graph
that keeps around descriptions for every node, and ensures that
nodes/edges are typed in accordance with the Odyssey plugin declaration.
In the future, I want to enable instances to declare their own node/edge
types, in which case the instance will assume responsibility for
tracking and serializing the types.
To make the Odyssey plugin a 'proper plugin', I've also added a plugin
declaration, as well as analysis and explorer adapters. I haven't
decided exactly where data for Odyssey instances should be stored, so
for now the plugin adapters always return an example instance which is
based on our experience at the Odyssey hackathon.
Test plan: The instance has unit tests for its logic.
If you want to see what the plugin looks like right now when it's
integrated, you can apply the following diff, and then load the
prototype. It will contain Odyssey nodes (find them using the node type
dropdown). Note that without a seed vector to move cred back to the
values/artifacts, the cred distribution in the Odyssey subgraph is
degenerate; the users are all sinks and have postiive cred scores, but
all the other nodes converge to 0 cred.
diff --git a/src/homepage/homepageExplorer.js b/src/homepage/homepageExplorer.js
index cae4548..48987f1 100644
--- a/src/homepage/homepageExplorer.js
+++ b/src/homepage/homepageExplorer.js
@@ -6,6 +6,7 @@ import type {Assets} from "../webutil/assets";
import {StaticExplorerAdapterSet} from "../explorer/adapters/explorerAdapterSet";
import {StaticExplorerAdapter as GithubAdapter} from "../plugins/github/explorerAdapter";
import {StaticExplorerAdapter as GitAdapter} from "../plugins/git/explorerAdapter";
+import {StaticExplorerAdapter as OdysseyAdapter} from "../plugins/odyssey/explorerAdapter";
import {GithubGitGateway} from "../plugins/github/githubGitGateway";
import {AppPage} from "../explorer/App";
import type {RepoId} from "../core/repoId";
@@ -14,6 +15,7 @@ function homepageStaticAdapters(): StaticExplorerAdapterSet {
return new StaticExplorerAdapterSet([
new GithubAdapter(),
new GitAdapter(new GithubGitGateway()),
+ new OdysseyAdapter(),
]);
}
This commit modifies `PagerankGraph.runPagerank` so that the user can
provide an alpha and seed vector. The seed vector is specified via a map
of weights, which will be normalized into a probability distribution
over all the nodes in the graph. In the event that the map is empty (or
the total weight is otherwise 0), a uniform distribution is created.
To effect this change, a helper function called `weightedDistribution`
has been added (and thoroughly tested) in the `graphToMarkovChain`
module. Then, that function is used in `pagerankGraph.runPagerank`
(along with light testing).
Currently, the default alpha is set to 0, to ensure consistency with the
legacy pagerank implementation in `analysis/pagerank`. Once that has
been replaced with `PagerankGraph`, we can consider changing the defualt
alpha to non-zero (thus removing the need for synthetic self-loops).
I took a different approach in the [odyssey-hackathon repo][commit].
The previous approach was a much more complicated (and fairly redundant)
API, that allowed specifying "NO_SEED", "UNIFORM_SEED", "SELECTED_SEED",
and "SPECIFIED_SEED". I'm much happier with this API and implementation.
[commit]: ed07861073
Test plan: Unit tests included; run `yarn test`.
Summary:
the cred calculation is defined by a Markov Mixing process. By
introducing the seed vector and teleportation parameter alpha, the
Markov mixing process is augmented with a source of cred originating
from the seed vector. The resulting algorithm is the generalized
variation of PageRank, allowing computation of both canonical PageRank
where the seed vector is the uniform distribution and personalized
PageRank where the seed vector is an indicator distribution. It is still
possible to get the simple markov chain solution by setting alpha = 0.
Note that this changes the Markov process state update, but does not
provide updates to the APIs. All existing behavior is unchanged because
alpha is always set to 0.
This is a port of
https://github.com/sourcecred/odyssey-hackathon/pull/3,
which was created during the Odyssey Hackathon.
Test Plan:
Existing tests have been extended to include passing alpha = 0 to
reproduce exisiting test cases for the simple Markov Process. Additional
test cases include
- Verifying that resulting stationary distribution is unaffected by seed when alpha = 0
- Verifying that resulting stationary distribution is precisely equal to seed when alpha = 1
- Verifying that the resulting stationary distribution is linear in the seed vector
- Verifying that the correct stationary distribution is computed for non-zero alpha
- verify that the algorithm converges immediately when the initialDistribution is the stationary distribution
- verify that the changing the initialDistribution does not change the stationary distribution
Paired with @mzargham
Right now PagerankGraph requires that the user choose specific values
for maxIterations and convergenceThreshold when running PageRank.
I also rename `PagerankConvergenceOptions` to `PagerankOptions`.
The motivation is that I want to add future arguments to the same
options dict (e.g. alpha and the seed vector), so the rename is
appropriate, and allowing the options to be unset (and thus inherit
default values) will make the whole API much cleaner as I add more
options.
Test plan: Unit test added. `yarn test` passes.
In [#1128: Add support for seed vectors][#1128], we significantly
increase the number of arguments to
markovChain.findStationaryDistribution. To clean up the invocations, I
added a followon PR (#1129) which converts findStationaryDistribution to
use a `PagerankParams` object instead.
However, I think it will be cleaner to land the PagerankParams refactor
before adding new features in #1128, so I'm making this PR as
pre-cleanup.
Test plan: This is a trivial refactor. `yarn test` passes.
[#1128]: https://github.com/sourcecred/sourcecred/pull/1128
This is a minor refactor so that we use Aphrodite for styling on
HomePage.js. It's not super consequential, but I want to switch to using
Aphrodite more consistently in the codebase, so why not start here.
Test plan:
`yarn test` reveals no errors.
`yarn start` launches a correctly styled frontend.
I also used `build_static_site.sh` and the resultant site is also
correctly styled.
Dandelion Mané
Tyler Mace
Brian Litwin