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

core is a singleton #76

Closed
wants to merge 1 commit into from
Closed

core is a singleton #76

wants to merge 1 commit into from

Conversation

eagmon
Copy link
Member

@eagmon eagmon commented Dec 18, 2024

This introduces a global singleton instance core within the process_bigraph module. The core is implemented as a singleton of the ProcessTypes class, ensuring that only one instance of ProcessTypes exists. The ProcessTypes class was modified to follow the Singleton design pattern using the __new__ method to ensure only one instance is created

Copy link
Contributor

@prismofeverything prismofeverything left a comment

Choose a reason for hiding this comment

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

This change successfully ensures that only a single instance of core exists in memory - however I don't think a singleton instance is appropriate for TypeSystem's role in the program. I can imagine use cases where we would want to be able to compute two different simulations in the same program which themselves have incompatible types registered in their respective core instances (a server that can run user-provided composites and display their results, for instance). Indeed, part of the goal of the system is to make the type system itself a state that can be computed by the system - this kind of self-reflection is only possible if new instances of the type definitions can be created and provided as context for further computations. This requirement alone requires the core instances be passed into whatever needs to access them.

I propose we continue with our dependency injection approach: most of the methods of core are called internally, and the user should in general only need to create one instance at initialization time and pass it into the outer composite.

@eagmon
Copy link
Member Author

eagmon commented Dec 18, 2024

They also need to pass the core into each process. Each process when created also needs to declare a core in the init, passed into super. When making tests we need to create a fixture for core. There is a whole game with how to register types in core within a repository and then pass that one into the composite. I would say over 50% of the time we are debugging we ourselves have found it is core-related. I would do a count of the number of times core is used in each repo, I am certain it is over 100 times. I have seen separate TypeSystem cores and ProcessType cores, which has also led to bugs. This has been mostly just us coming upon these issues, not even the general public. When I am trying to onboard people, this becomes a thing every time.

Is all of this worth supporting a possible future hypothetical scenario with multiple incompatible type systems? It is actually hard to imagine this future scenario as being useful at all, I personally can't even think of an example in which we would use this -- can't we just have two projects if we want incompatible systems? Are they incompatible because of their methods conflicting with each other (hard to imagine what would make them incompatible). To me it doesn't seem this complex future hypothetical is worth all the issues mentioned in the paragraph above. I'm mostly trying to lower people's activation energy to understanding this repo and using it. Core is a major hurdle, and I don't think they should even have to know about it honestly for 100% of scenarios that we have ever discussed.

@eagmon eagmon closed this Dec 21, 2024
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.

2 participants