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

Circular dependency between IonChannelMechanism and SubCellularModel #173

Open
wizmer opened this issue Aug 24, 2018 · 7 comments
Open

Circular dependency between IonChannelMechanism and SubCellularModel #173

wizmer opened this issue Aug 24, 2018 · 7 comments

Comments

@wizmer
Copy link
Contributor

wizmer commented Aug 24, 2018

Hi,

IonChannelMechanism has a property subCellularModel that targets a SubCellularModel and the later has a property isPartOf that target one of IonChannelMechanism or SynapseRelease.

In fact the property isPartOf is the only distinction between SubCellularModel and SubCellularModelScript. That make me think that SubCellularModel could actually be removed.

Indeed IonChannelMechanism and SynapseRelease could directly points to the SubCellularModelScript nodes without having to pass through SubCellularModel and without any loss of information.

What do you think ?

@wizmer
Copy link
Contributor Author

wizmer commented Aug 24, 2018

Pinging my boss @genric :)

@genric
Copy link
Collaborator

genric commented Aug 24, 2018

I'm fine with removing it, just a bit worried that we might discover some properties in the future which will make sense to attach to SubCellularModel and will have to reintroduce that entity.

@MFSY
Copy link
Contributor

MFSY commented Aug 24, 2018

I would like to add @apdavison on this.

@MFSY
Copy link
Contributor

MFSY commented Aug 24, 2018

Hi @wizmer,

Do you have SubCellularModel data ? If it is a valid entity that exists in your pipeline, I suggest to keep it. I think the main difference between SubCellularModel and SubCellularModelScript is they are different type of entities.

@wizmer:

I guess you're talking about IonChannelMechanismReleaseShape that targets nsg:IonChannelMechanismRelease. I was not able to find IonChannelMechanism.

What I've seen so far is:
nsg:IonChannelMechanismRelease --nsg:subCellularModel--> nsg:SubCellularModel
nsg:SubCellularModel --dcterms:isPartOf--> nsg:IonChannelMechanismRelease

I don't call this circular dependancy as the property (nsg:subCellularModel) that link a nsg:IonChannelMechanismRelease to a nsg:SubCellularModel is different to the one (dcterms:isPartOf) that link a nsg:SubCellularModel to a nsg:IonChannelMechanismRelease.

Also only neurosciencegraph/simulation/ionchannelmechanismrelease/v0.1.3 imports neurosciencegraph/simulation/subcellularmodel/v0.1.3.

But I suggest the following changes:

  • Let remove the import from ionchannelmechanismrelease to subcellularmodel because you'll not be able to validate data against the schema ionchannelmechanismrelease. If the validation currently passes it is just because nothing is mandatory in the subcellularmodel schema.
    Also the validation tool used in Neuroshapes does not support multiple target declaration within the scope of a schema. In the scope of the ionchannelmechanismrelease schema there are two targetClass declarations:

    **"targetClass": "nsg:IonChannelMechanismRelease" defined in ionchannelmechanismrelease schema
    ** "targetClass": "nsg:SubCellularModel" defined in subcellularmodel schema

  • if the above import is removed than the subCellular property shape can be changed (in ionchannelmechanismrelease schema) as follows:

From:

{
              "path": "nsg:subCellularModel",
              "name": "Subcellular model",
              "description": "Subcellular models which are part of this release.",
              "node": "{{base}}/schemas/neurosciencegraph/simulation/subcellularmodel/v0.1.3/shapes/SubCellularModelShape"
}

To:

{
              "path": "nsg:subCellularModel",
              "name": "Subcellular model",
              "description": "Subcellular models which are part of this release.",
              "class": "nsg:SubCellularModel"
}

Not that the node ref is replaced by a type check (class).

@wizmer
Copy link
Contributor Author

wizmer commented Aug 24, 2018

Yes, I was talking about IonChannelMechanismRelease and no, I have no SubCellularModel data.

Well, yes it may not be purely cyclic as the properties have different names but they still convey exactly the same meaning (ie. the container (IonChannelMechanismRelease) <---> contained object (SubCellularModel) relationship).
Also, isPartOf looks a bit weird to me as it defines a constraint not on the object itself but on its potential containers. If follows that any inclusion of a SubCellularModel object into a new container class would require an update on subCellularModel schema itself.

Sorry if it is obvious to you but I don't understand how does your suggestion solve the issue. Let's say you want to create a IonChannelMechanismRelease object, you still need to first create a SubCellularModel first, who itself require the IonChannelMechanismRelease object in order to be created ?

That being said, I'm a total noob in the Nexus world and probably don't see all the implication of removing the SubCellularModel class (maybe other people are using it ?)

@MFSY
Copy link
Contributor

MFSY commented Aug 24, 2018

Let's say you want to create a IonChannelMechanismRelease object, you still need to first create a SubCellularModel first, who itself require the IonChannelMechanismRelease object in order to be created ?

No: because nothing is mandatory (there is no minCount set to 1) in both schemas. You can create a IonChannelMechanismRelease object without a SubCellularModel model and you can create a SubCellularModel without a IonChannelMechanismRelease. Again that's what I read from the schemas. The way the schemas are written need to be changed if you want to make mandatory fields.

Well, yes it may not be purely cyclic as the properties have different names but they still convey exactly the same meaning (ie. the container (IonChannelMechanismRelease) <---> contained object (SubCellularModel) relationship).

I don't think they convey the same meaning but I see your point. In linked data and SHACL it is quite common to have two entities referencing each other.

Also, isPartOf looks a bit weird to me as it defines a constraint not on the object itself but on its potential containers. If follows that any inclusion of a SubCellularModel object into a new container class would require an update on subCellularModel schema itself.

This is a valid point. The answer of the following question can help here:

  • How many subcellularmodel do you have per ionchannelelease ?
    If it is a big number then it make sense to have a link from subcellularmodel to ionchannelelease because I think you don't want to have a giant array of subcellualmodels. If the answer is a small number then a link from ionchannelelease to subcellularmodel make more sense.

@wizmer
Copy link
Contributor Author

wizmer commented Aug 24, 2018

Thanks a lot for your answers @MFSY , that helps a lot.
I see, so yes, if I do not provide any model while creating IonChannelMechanismRelease I'll be good.
Actually @genric suggested that we could completely drop to avoid such confusion (although you're saying it's quite common in the SHACL world) :

{
              "path": "nsg:subCellularModel",
              "name": "Subcellular model",
              "description": "Subcellular models which are part of this release.",
              "node": "{{base}}/schemas/neurosciencegraph/simulation/subcellularmodel/v0.1.3/shapes/SubCellularModelShape"
}

And for information, the IonChannelRelease I'm using has around ~30 models.
Thanks again

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