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

deterministic graph execution #498

Open
XzzX opened this issue Nov 20, 2024 · 1 comment
Open

deterministic graph execution #498

XzzX opened this issue Nov 20, 2024 · 1 comment
Labels
question Further information is requested

Comments

@XzzX
Copy link
Contributor

XzzX commented Nov 20, 2024

def fetch(self) -> None:
"""
Sets :attr:`value` to the first value among connections (i.e. the most recent)
that is something other than `NOT_DATA`; if no such value exists (e.g. because
there are no connections or because all the connected output channels have
`NOT_DATA` as their value), :attr:`value` remains unchanged.
I.e., the connection with the highest priority for updating input data is the
0th connection; build graphs accordingly.
Raises:
RuntimeError: If the owner is :attr:`running`.
"""
for out in self.connections:
if out.value is not NOT_DATA:
self.value = out.value
break

Is this a feature? This makes graph execution non deterministic, does it?

@XzzX XzzX added the question Further information is requested label Nov 20, 2024
@liamhuber
Copy link
Member

It is a feature, and for the current implementation of the "while" flow of information it's a critical one. We decided at the meeting that it would be sensible to switch to only allowing each input to have at most one connection (still arbitrary connections for output, of course).

I agree it muddies graph execution. I guess other stuff does too, like the If node, but I'm fully on-board with making it more deterministic here. After formulating the new while loop (#501), and restricting the number of input connections (#502), the only remaining bit to close this issue is to always fetch the upstream data even if you currently have a data-like value and upstream doesn't. AFAIK the only place this "feature" is leveraged is in the while-loop stuff, so once #501 is done, I don't anticipate the further changes to close this having any impact on the test suite or anybody's applications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants