Feature/sckan 275 #250

afonsobspinto · 2024-03-16T00:06:31Z

Closes https://metacell.atlassian.net/browse/SCKAN-275

Updates the django models to support region + layer as discussed here
Adds migration to move from the old model to a new model without losing data. The migrations are split into 4 steps:

Add the new models and new properties to the models that are going to be affected (Connectivity Statement, VIas and Destinations)
Copy the old properties of the models affected to the new properties created in 1 (f.e copy ConnectivityStatement.origins to ConnectivityStatement.origins_new)
Remove old AnatomicalEntity model and properties that referred to it
Rename new properties to the names used previously

Updates django admin page to add basic support for the new region + layer model (might be enough to close https://metacell.atlassian.net/browse/SCKAN-279 but I didn't put any thought into it, I just added what I need for testing purposes cc @D-GopalKrishna )
Updates serializers to be compliant with the new model just enough for the application doesn't crash ( it's possible that this will change again in the scope of https://metacell.atlassian.net/browse/SCKAN-276)
Updates command to ingest anatomical entities
Updates command to ingest statements from neurondm. It includes a small refactoring that splits different responsibilities of the script per different files. The neurondm script was changed to keep the region + layer information. The cs ingestion service was changed to be compliant with the new model and also to allow the 'moving' a generic anatomical entity (meta) into a specific (region or layer) anatomical entity (meta) when the flag update_anatomical_entities is set. Updates changes detector algorithm and validators to work with complex entities.
Generates new openapi specification and frontend stubs
Modifies the frontend just enough for the application not crash on the most basic functionalities (requires further testing cc @ddelpiano )

afonsobspinto · 2024-03-16T00:10:15Z

backend/composer/models.py

@@ -229,7 +228,7 @@ class AnatomicalEntity(models.Model):
    """Anatomical Entity"""

    name = models.CharField(max_length=200, db_index=True)
-    ontology_uri = models.URLField(unique=True)
+    ontology_uri = models.URLField()


This can't be unique because we can have children of anatomical entities with the same ontology uri

afonsobspinto · 2024-03-16T00:13:44Z

backend/composer/models.py

+
+class Region(AnatomicalEntity):
+    ...
+    associated_layer = models.ForeignKey(Layer, on_delete=models.CASCADE, related_name='regions')


I had to make this a ForeignKey instead of a ManyToMany because when we create a connectivity statement with a region-layer pair we need to know exactly what is the region-layer pair and we expect that information to be under the umbrella of the anatomical entity model. In other words, if we want to add a complex origin to a connectivity statement we need to add an anatomical entity that contains information about the specific region and the specific layer.

@afonsobspinto I'm not sure I follow your comment.
I remember that a region can have multiple layers, how would this look like in the model you suggest?

@zsinnema A region can have multiple layers yes, but only a pair region layer represents an anatomical entity. For example:

Region: Segment L1
Layers: Lamina 1, Lamina 2, Lamina 3, Lamina 4, etc...

The anatomical entities that we have, as far as I understand, are:

Segment L1 + Lamina 1

Segment L1 + Lamina 2

....

One relevant point to consider too is that in the Connectivity Statement model we refer to AnatomicalEntity (not RegionLayerPair).
origins = models.ManyToManyField(AnatomicalEntity, related_name='origins_relations')

afonsobspinto · 2024-03-17T22:21:13Z

I think it might make more sense to have anatomical entities as an abstract model and have as children:

Simple Anatomical Entity
Layer
RegionLayer

zsinnema · 2024-03-18T08:53:46Z

backend/composer/models.py

+
+class Region(AnatomicalEntity):
+    ...
+    associated_layer = models.ForeignKey(Layer, on_delete=models.CASCADE, related_name='regions')


@afonsobspinto I'm not sure I follow your comment.
I remember that a region can have multiple layers, how would this look like in the model you suggest?

…asic functionalities only)

afonsobspinto · 2024-03-20T19:09:48Z

I updated the description of the PR accordingly to the most recently changes.

For tracking purposes, this is what the original description was:

Updates composer model to have both region and layer as anatomical entities
Update neuronDM script to retain region + layer information
Update ingest statements command to create region and layers when needed.

zsinnema

@afonsobspinto I;ve been playing with the admin and for now I think it's not usable anymore.
Before the admin gave a clear overview of the KS and it's properties and linked data

In the new situation, for example look at the Destinations, the Admin interface is unusable.

Also the speed of the Admin interface went down dramatically.

Please have a closer look at the usability and speed.
Thanks

* #277 add ontology_uri property to AnatomicalEntity * #277 - simplify the @Property for name and ontology_uri in AE

afonsobspinto · 2024-04-02T10:01:08Z

@zsinnema I did not had the time to try any of the speed improvement suggestions on the admin page yet. Would it be possible to merge this PR only with the usability changes so that @ddelpiano can test it?

zsinnema · 2024-04-02T11:37:29Z

@afonsobspinto imo this is not mergable, see the attached video. In this video I open a (one) ConnectivityStatement in the admin using this link https://127.0.0.1:8000/admin/composer/connectivitystatement/296/change/ (I ingested the dev database)

sckan.pg.queries.mp4

afonsobspinto · 2024-04-02T11:55:35Z

@zsinnema Do you believe this is not mergeable do to the high amount of queries?
If that's so, I agree with you that it's something that could be improved but my point is that it's not something that is responsibility of this PR. Have a look at the SQL query count in both dev and local with dev database:

Dev:

Local with dev database:

zsinnema · 2024-04-02T12:09:18Z

@afonsobspinto herewith a video with the dev db on my local machine and git branch develop.
as you can see the amount of queries is a "million" times less

Ubuntu.2024-04-02.14-07-58.mp4

zsinnema · 2024-04-02T12:34:27Z

@ddelpiano The performance decreased is caused by the chanegs done in this PR/feature. Therefore I classify the implementation as being not mergable. I understand that the client also needs to see things/progress so the question is: how big is the is the performance decrease for the usability of the application.
In the past we faced a perf decrease and then it was a big thing.
Of course we could say that the performance decrease is a "bug". But the thing is that we already know the bug and the PR isn't merged yet.

Up to you if you think we can merge it or not

afonsobspinto · 2024-04-02T12:38:01Z

@zsinnema I can't replicate the behaviour of having such high amount of queries when running with this branch with the dev database:

2024-04-02.13-27-04.mp4

This is what I did:

docker-compose -f docker-compose-db.yaml up -d
cat composer.sql | docker exec -i 30c8dea28dc9 psql -U <User> -d <Password>
USE_PG=1;DEBUG=1 python manage.py migrate
USE_PG=1;DEBUG=1 python manage.py ingest ingest_statements --update_upstream --update_anatomical_entities
USE_PG=1;DEBUG=1 python manage.py ingest runsslserver

zsinnema · 2024-04-02T12:52:20Z

@afonsobspinto hmmm after I pulled again from Git the many queries disappeared... weird

SCKAN-275 feat: Add layer and region models

e88b3b1

afonsobspinto commented Mar 16, 2024

View reviewed changes

afonsobspinto force-pushed the feature/SCKAN-275 branch from b8eae5e to 1fbe45c Compare March 16, 2024 00:19

SCKAN-275 feat: Update neurondm ingestion

416f255

afonsobspinto force-pushed the feature/SCKAN-275 branch from 1fbe45c to 416f255 Compare March 16, 2024 00:23

afonsobspinto requested a review from zsinnema March 17, 2024 22:21

afonsobspinto marked this pull request as ready for review March 17, 2024 22:21

zsinnema requested changes Mar 18, 2024

View reviewed changes

Merge branch 'feature/SCKAN-274' into feature/SCKAN-275

31e88aa

afonsobspinto changed the base branch from feature/SCKAN-274 to develop March 19, 2024 13:53

afonsobspinto marked this pull request as draft March 19, 2024 13:53

afonsobspinto added 4 commits March 19, 2024 15:00

SCKAN-275 feat: WIP - Update anatomical entity model

86a7c3a

SCKAN-275 feat: WIP - Remove old anatomical entity model

de97874

SCKAN-275 feat: Connect new anatomical entity model

96ffa1e

SCKAN-275 feat: Add basic admin panel controls

776237e

afonsobspinto force-pushed the feature/SCKAN-275 branch from 253d241 to 17447c2 Compare March 20, 2024 13:07

SCKAN-275 feat: Update models

40f20b9

afonsobspinto force-pushed the feature/SCKAN-275 branch from 17447c2 to 40f20b9 Compare March 20, 2024 14:31

afonsobspinto added 3 commits March 20, 2024 14:46

SCKAN-275 feat: Add basic serializers

75a980b

SCKAN-275 refactor: Split ingestion script in multiple modules

87633da

SCKAN-275 feat: Update changes detector

0ce72d8

afonsobspinto force-pushed the feature/SCKAN-275 branch from cb07d67 to 0ce72d8 Compare March 20, 2024 17:45

afonsobspinto added 4 commits March 20, 2024 18:08

SCKAN-275 feat: Update validators

bce80aa

SCKAN-275 feat: Update ingest anatomical entity script

ec80e8b

SCKAN-275 fix: Fix typo

3ce0636

SCKAN-275 feat: Update frontend to be compliant with model changes (b…

318eb51

…asic functionalities only)

afonsobspinto requested a review from zsinnema March 20, 2024 19:29

afonsobspinto marked this pull request as ready for review March 20, 2024 19:29

afonsobspinto and others added 5 commits March 21, 2024 13:57

SCKAN-275 feat: Update Anatomical Entity Admin page

7b735ed

SCKAN-275 feat: Add automatic creation of anatomical entities

fadd3aa

SCKAN-275 feat: Update admin page names

f40300c

SCKAN-275 feat: Add anatomical entity constraint

3e043ea

chore(): added feature to use a local PostgreSQL database

a7daca8

zsinnema reviewed Mar 22, 2024

View reviewed changes

afonsobspinto and others added 2 commits March 27, 2024 12:29

SCKAN-275 feat: Set name as string representation of AnatomicalEntity

99c0245

Feature/csckan-277 (#252)

2bd433a

* #277 add ontology_uri property to AnatomicalEntity * #277 - simplify the @Property for name and ontology_uri in AE

afonsobspinto requested a review from zsinnema April 2, 2024 09:52

zsinnema approved these changes Apr 2, 2024

View reviewed changes

zsinnema merged commit 19efdf4 into develop Apr 2, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/sckan 275 #250

Feature/sckan 275 #250

afonsobspinto commented Mar 16, 2024 •

edited

Loading

afonsobspinto Mar 16, 2024

afonsobspinto Mar 16, 2024

zsinnema Mar 18, 2024

afonsobspinto Mar 18, 2024

afonsobspinto commented Mar 17, 2024

zsinnema Mar 18, 2024

afonsobspinto commented Mar 20, 2024

zsinnema left a comment

afonsobspinto commented Apr 2, 2024

zsinnema commented Apr 2, 2024 •

edited

Loading

afonsobspinto commented Apr 2, 2024 •

edited

Loading

zsinnema commented Apr 2, 2024

zsinnema commented Apr 2, 2024

afonsobspinto commented Apr 2, 2024 •

edited

Loading

zsinnema commented Apr 2, 2024

Feature/sckan 275 #250

Feature/sckan 275 #250

Conversation

afonsobspinto commented Mar 16, 2024 • edited Loading

afonsobspinto Mar 16, 2024

Choose a reason for hiding this comment

afonsobspinto Mar 16, 2024

Choose a reason for hiding this comment

zsinnema Mar 18, 2024

Choose a reason for hiding this comment

afonsobspinto Mar 18, 2024

Choose a reason for hiding this comment

afonsobspinto commented Mar 17, 2024

zsinnema Mar 18, 2024

Choose a reason for hiding this comment

afonsobspinto commented Mar 20, 2024

zsinnema left a comment

Choose a reason for hiding this comment

afonsobspinto commented Apr 2, 2024

zsinnema commented Apr 2, 2024 • edited Loading

afonsobspinto commented Apr 2, 2024 • edited Loading

zsinnema commented Apr 2, 2024

zsinnema commented Apr 2, 2024

afonsobspinto commented Apr 2, 2024 • edited Loading

zsinnema commented Apr 2, 2024

afonsobspinto commented Mar 16, 2024 •

edited

Loading

zsinnema commented Apr 2, 2024 •

edited

Loading

afonsobspinto commented Apr 2, 2024 •

edited

Loading

afonsobspinto commented Apr 2, 2024 •

edited

Loading