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

ChannelModel class returns ReceivesSquareCurrent as failed capability for EGL-19.channel model #221

Open
NeZanyat opened this issue May 31, 2019 · 7 comments

Comments

@NeZanyat
Copy link
Contributor

Url to file https://raw.githubusercontent.com/openworm/ChannelWorm2/master/NML2_models/EGL-19.channel.nml

@gidili
Copy link

gidili commented May 31, 2019

@rgerkin this makes it impossible for us to create an instance of ChannelModel with this file. Something you can fix?

@rgerkin
Copy link
Contributor

rgerkin commented May 31, 2019

@gidili

tl;dr I should just remove "ReceivesSquareCurrent" as a capability for this model class, because it isn't really even doing that! It is just actually doing voltage steps. And I'm not sure what those capability checks would even look like, see below:

I realize now that in order to run this I was using pyNeuroML's channel analyzer module, which takes this .nml file and wraps it with a LEMS template that provides the ability to inject current. I put the resulting LEMS file [here]https://github.com/scidash/neuronunit/blob/dev/neuronunit/examples/model_zoo/LEMS_Test_ca_boyle.xml) and you will see that it had the original .nml file as an include.

Naturally when you check for extra capabilities on the site from this .nml file alone, they will fail because the ability to inject current has not been added, i.e. there is no way in the .nml file on its own to inject current.

Ideally, the XML template in pyNeuroML would set up current injection via a pulseGenerator tag as in the Izhikevich example, in which case the capability check would then succeed (on the .xml file), but instead it does so by defining a "vClampedCell" which has its own LEMS-defined dynamics, and I don't think it is realistic to parse those dynamics to determine whether a cell has a certain capability. However, it might be sufficient to just check for a vClampedCell element as an alternative to a pulseGenerator tag, and hope that there is some convention, at least for pyNeuroML, of using such an element in this type of simulation experiment. This could be the basis of a "ReceivesSquareVoltage" capability in the future.

But for now I think I should just remove that capability because its implementation is really tied up in the model file in ways that I should coordinate with pyNeuroML people (i.e. Padraig) before trying to solve one specific case.

What do you think?

@NeZanyat
Copy link
Contributor Author

NeZanyat commented Jun 20, 2019

A bit more things:

  1. backend parameter is missing here https://github.com/scidash/neuronunit/blob/dev/neuronunit/models/channel.py#L15 but it's required because in other case it will cause an error

  2. We still have to remove this capability https://github.com/scidash/neuronunit/blob/dev/neuronunit/models/lems.py#L26

@rgerkin
Copy link
Contributor

rgerkin commented Jun 20, 2019

@NeZanyat
I've implemented (1) in 94f3202 but I don't understand why (2) needs to be removed for ChannelModel to work. The extra_capability_checks only apply to capabilities claimed by the model, i.e. it will only fail if the a capability which is a key in extra_capability_checks is also a parent class of the model. Since ChannelModel does not have ReceivesSquareCurrent as a capability, then it shouldn't fail this check. Am I mistaken, or are you talking about a different model?

@gidili
Copy link

gidili commented Jun 21, 2019

Hi @rgerkin ChannelModel itself doesn't have ReceivesSquareCurrent as a capability but inherits from LEMSModel that does have it as an extra capability check, so we are still finding that capability showing up as a failed extra capability with the same model when we call this method. Are we missing something?

@rgerkin
Copy link
Contributor

rgerkin commented Jun 21, 2019

@NeZanyat @gidili OK, I see. failed_extra_capabilities was too strict. extra_capability_checks may have many Capability:function mappings, but these should only be checked against Capabilities actually claimed by the model. LEMSModel.extra_capability_checks provides such a mapping for ReceivesSquareCurrent, but if a model does not claim the ReceivesSquareCurrent capability than this check should be ignored.

In the notebook or command line, I never call failed_extra_capabilities because I don't have a need to, which is why the ChannelModel example worked for me. But since you need it for the website, I have made the change in scidash/sciunit@06d84c5, which checks to see if the model actually claims the capability (i.e. is a subclass of it) before reporting that it failed. Let me know if this fixes the problem.

@gidili
Copy link

gidili commented Jun 22, 2019

Thanks @rgerkin that'll definitely do it. We will give this a shot asap and report back! 🙏

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

3 participants