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

feat: add DatabaseEmitter implementation class #33

Merged
merged 113 commits into from
Dec 16, 2023
Merged

Conversation

AlexPatrie
Copy link
Contributor

@AlexPatrie AlexPatrie commented Nov 25, 2023

What does this PR do?:

  • feat: ports over and implements DatabaseEmitter class from vivarium-core, complete with all abstract methods in process-bigraph
  • feat: adds utils functions to emitter.py file interacting with aforementioned object.
  • docs: adds type annotations wherever possible, complete with doc string representation
  • chore: refactors testing directory and associated files complete with test model files.
  • feat: adds example processes to experiments library

Copy link
Member

@eagmon eagmon left a comment

Choose a reason for hiding this comment

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

@AlexPatrie this is fantastic, thank you! Did you get this to work as-is?

One comment -- the Emitter base class is a type of Step and gets added to the Composite just like any other process would, so I think we need to add the schema method for this to work. Steps/Processes have inputs that they read and then perform the emit in whatever form. Here is an example of how an emitter gets wired into the composite:

'emitter': {
'_type': 'step',
'address': 'local:ram-emitter',
'config': {
'ports': {
'inputs': {
'mRNA': 'tree[float]',
'interval': 'float'}}},
'wires': {
'inputs': {
'mRNA': ['mRNA'],
'interval': ['event', 'interval']}}}}}

process_bigraph/emitter.py Show resolved Hide resolved
process_bigraph/emitter.py Outdated Show resolved Hide resolved
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.

Good to get all this in here, thanks @AlexPatrie! Will need to be adapted a bit for the new emitter structure (emitters are just steps now) but it's a good start.

process_bigraph/emitter.py Show resolved Hide resolved
Copy link
Member

@eagmon eagmon left a comment

Choose a reason for hiding this comment

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

This looks good, let's get the core processes out to a separate repo and then this is good to merge. Thanks!

)


class CopasiProcess(Process):
Copy link
Member

Choose a reason for hiding this comment

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

move to core-processes

@AlexPatrie AlexPatrie merged commit 800183b into main Dec 16, 2023
1 check passed
@AlexPatrie AlexPatrie deleted the emitter-adding branch December 16, 2023 02:31
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.

3 participants