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

Bug: Resource report has wrong stage? #186

Open
fosterlynn opened this issue Oct 30, 2022 · 3 comments
Open

Bug: Resource report has wrong stage? #186

fosterlynn opened this issue Oct 30, 2022 · 3 comments

Comments

@fosterlynn
Copy link
Collaborator

On the following report, the haralson apple pie resource says "(Fill pie)", which is a process spec. Is this supposed to be the stage of the resource? If so, it has not changed after the pies were baked, which would be a bug. If it is not the stage, I'd suggest that the most useful data there is the stage and we should change the report.

Screenshot from 2022-10-30 17-40-56

This is the last event.

Screenshot from 2022-10-30 17-28-28

@adaburrows
Copy link
Member

adaburrows commented Dec 31, 2022

Oh, it's entirely possible that this comment was prescient (I wrote it with the accounting code):

  // Sort events by time of creation, but topological ordering is preferable
  const sortedEvents = economicEvents.sort((a, b) => +b.created - +a.created);

Basically, if you don't create the events in the order they need to be executed, then you will end up with the wrong state. This is because the operation of addition is commutative and associative, but the algebra of string operations is not commutative: "Hello " + "world!" !== "world!" + Hello " or "Hate", del(3), ins(1, "e"), ins(3, "r") => "Heart" while "Hate", ins(3, "r"), ins(1, "e"), del(3) => "Heare"

To fix this, we either need to ensure the hasPointInTime is filled and used to order all events (this will require form validation) —OR— we need to ensure the algorithm traverses the graph from the first EconomicEvent that creates the EconomicResource through all the processes and transfer events until the last event that effects the specific EconomicResource.

Since we know that only the stage from the last EconomicEvent needs to be applied, we can apply the full last event and then do accounting backwards to the first event that creates it (since addition is commutative). Having a way to tie the creation of the resource to a specific event would be good because otherwise we would have to rely on the creation date of the resource or query all events that reference the resource and the processes to which those events refer to and traverse the entire history that relates to the resource. Another optimization my be gained by recording the events that made the resource quantity equal zero, then we would only need to walk backwards to that point. Though, I'm going to assume that in the playspace, this will never be that big of a deal because I don't foresee millions of events being stored in it for for a few years.

Even if we switched this over to doing the accounting once and making it "permanent", there's always the chance another event will be created that would change the stage in the past. If that happens, then we're still back to needing to evaluate the whole chain in the correct topological order each time accounting is done.

This might also effect hREA since it seems to apply the accounting in Holochain action order (though I might not have read through all the applicable code).

I may just move this and a series of other issues that require the graph traversal into another milestone.

@fosterlynn
Copy link
Collaborator Author

Yeah, we'll need the graph traversal, we can't count on creation time (race conditions, wrong server times, people just entering things later, etc.) or even the event time(s) for some of the same reasons. See #180 (comment).

This might also effect hREA since it seems to apply the accounting in Holochain action order (though I might not have read through all the applicable code).

Interesting, since most of the accounting doesn't care (increment/decrement). But stage cares, taking a note to make sure VF says something about that.

@adaburrows
Copy link
Member

adaburrows commented Dec 31, 2022 via email

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

No branches or pull requests

2 participants