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

Formula setup hash #98

Merged
merged 3 commits into from
Apr 23, 2017
Merged

Formula setup hash #98

merged 3 commits into from
Apr 23, 2017

Conversation

warpfork
Copy link
Member

Rename Formula.Hash to Formula.SetupHash to clarify what it is, why it does what it does, and that it does not include coverage of all fields.

Update many docs that previously referred to a 'conjecture' field, and remove that field, as part of a clarification sweep. Those docs now make references to SetupHash where appropriate (and read much better as a result, imo).

Signed-off-by: Eric Myhre <hash@exultant.us>
Reasoning and purpose is as already described in the docs attached to the method.  This rename should reduce confusion and leaves the air clearer for when we add other hash functions that produce other kinds of useful index/ID.

Signed-off-by: Eric Myhre <hash@exultant.us>
@warpfork warpfork force-pushed the formula-setup-hash branch 3 times, most recently from d13650b to 82cf61c Compare April 19, 2017 11:52
…r to this concept.

This is being removed because it's not exactly the correct place for that concern, and worse, leaving the mentions there has proved very misleading.  Whether or not an output is consistent doesn't have much to do with how we run things.  Something should check that, sure; but how, and against what matrix you want to check it is a question that has more than one correct answer, and it *definitely* doesn't have anything to do with how we run something.

The Formula.SetupHash is a related concept, despite not actual covering results -- or perhaps because it specifically DOES NOT cover results, but DOES cover output slot specifications.  Most docs previously refering to the conjecture now make more sense referring to the SetupHash instead.

There is no function which currently computes a Formula.ResultsHash, which sounds like a function that would make more sense to concern itself with conjectures.  This doesn't exist largely because downstreams (namely, reppl, which is an example of a practical tool that's stitching things together and really cares about memoizing, etc) have so far been getting along fine by looking at each line of result hash separately.  In other words, there's no reason this function doens't exist; just that it hasn't been needed yet.  If we define one (and to be clear: it will not replace SetupHash; it will be ADifferentHash), then a conjecture-like field may make an appearance again, but until that time, removing it seems best.

Signed-off-by: Eric Myhre <hash@exultant.us>
@warpfork warpfork force-pushed the formula-setup-hash branch 2 times, most recently from d0785f1 to 7655d89 Compare April 19, 2017 16:30
Copy link
Member

@TripleDogDare TripleDogDare left a comment

Choose a reason for hiding this comment

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

lgtm

@warpfork warpfork merged commit 0e283b4 into master Apr 23, 2017
@warpfork warpfork deleted the formula-setup-hash branch April 23, 2017 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants