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

Should Pump and Channel be separate? #592

Open
michaeldeistler opened this issue Feb 25, 2025 · 2 comments
Open

Should Pump and Channel be separate? #592

michaeldeistler opened this issue Feb 25, 2025 · 2 comments

Comments

@michaeldeistler
Copy link
Contributor

michaeldeistler commented Feb 25, 2025

Follow-up to #438

How should we deal with Pumps? Currently, they are separate from the Channel. We could also merge them and have something like self.ion_name=None (or self.ion_name = "v") for Channel`s.

Currently, we define self.pumps and self.pumped_ions. As @jnsbck suggested, we could also define self.pumps = {"ion_name": [pump1, pump2]}. If we merge Pumps and Channels, we could even have:

self.mechanisms = {"v": [channel1, channel2], "ca": [pump1], "na": [pump2]}

A downside of this is that

  1. by removing the Pump, we lose the opportunity to write explicit docstrings that explain what a Pump is
  2. we hide that a Pump and a Channel differ in their units and in the fact that channels get the current divided by the capacitance.

A final option would be to only go for the dict representation for Pumps. This has the downside that:

  1. we now handle pumps and channels differently
  2. we can no longer do self.channels + self.pumps

But it has the big advantage of having only one attribute (get rid of self.pumped_ions)

@jnsbck
Copy link
Contributor

jnsbck commented Feb 25, 2025

I would still opt to have Pump even if Pump is just class Pump(Channel) (or class Pump(Mechanism) see #474 (comment)) and calls super().__init__(self,...).

I like keeping mechanisms in one place, with self.mechanisms, in this case we'd have to incl. synapses as well. Possible would also be self.mechanisms = {"synapses": {}, "pumps": {}, "channels": {}}

I find self.ion_name=None/v, confusing. In that case Id rename it to modified_state/affectsor sth., but it would be nice to have this be an attr ofMechanism` and therefore all child mechanisms. The more unified we can make this, the easier it will probably be to handle this in the backend, the more room for optimizations etc. there will be.

@michaeldeistler
Copy link
Contributor Author

michaeldeistler commented Feb 26, 2025

We just discussed. We will have:

class Mechanism:
    def derivative():
        pass

class Channel(Mechanism):
    def compute_current():
        pass

class Synapse:
    def compute_current():
        pass

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