Why are `running` and `failed` still coexisting?

[samwaseda]

I remember to have talked about it some time ago, but doesn't it make more sense to do like in pyiron_base to define one class for the states, like initialised, running, failed, finished? If I understand the current implementation correctly, it is technically not impossible to set both running and failed to true, but it should be logically impossible right? Also I find it difficult to understand what are possible states, because running and failed are listed among a lot of items on the base level of Node.

[liamhuber]

The short answer is: yes, there should absolutely be an abstraction coupling running and failed and making them mutually exclusive. But it's not broken as-is and doing this has been low-priority enough that it just hasn't happened.

doesn't it make more sense to do like in pyiron_base to define one class for the states, like initialised, running, failed, finished?

I like the way base has a state object that shows these attributes are all connected and makes them mutually exclusive, but I don't like it enough to just copy-paste it over here. They also don't all apply:

• initialised: doesn't even make sense here, nodes are much simpler than jobs
• running: yep
• failed: yep
• finished: We could have it, but I'm not convinced it makes as much sense. You can re-run nodes multiple times, so it's not really mutually exclusive with initialised, and I don't know that it's meaningful to track if it's been run. Also it seems very redundant to node.outputs.ready

it is technically not impossible to set both running and failed to true, but it should be logically impossible right?

Correct on all counts! But unless you run into a case where they are both true, fixing it will remain low-priority for me.

Also I find it difficult to understand what are possible states, because running and failed are listed among a lot of items on the base level of Node.

That's totally fair. They're the only two states in their mutually exclusive group. If mutual exclusion wasn't how we're grouping things, stuff like ready and import_ready could also be included in the state-ness collection.

I've tried super hard to keep "because history" out of the explanations for the codebase here, but honestly this is one place where it snuck in. I started with just failed as a bool, later introduced running as a bool but didn't have time to abstract them together, and then just stopped touching that corner of the code base for a long, long time. I totally agree an abstraction would be nice here, but I'm not sure I'll get it (and I might even make things slightly worse in the short term trying to get the functionality of #458 merged).