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

Soma contour centroid oriented so 1 end is largest positive direction. #2491

Merged
merged 5 commits into from
Oct 12, 2023

Conversation

nrnhines
Copy link
Member

@nrnhines nrnhines commented Sep 7, 2023

A temporary pull request to allow comparison with #2470
This and the cornu/add_eigen branch have the same method for disambiguation of the orientation of the centroid.

Soma contour centroid oriented so 1 end is largest positive direction.

Therefore simulations using neurolucida asc files should produce the same results.

@azure-pipelines
Copy link

✔️ 8330f7a -> Azure artifacts URL

@nrnhines
Copy link
Member Author

nrnhines commented Sep 8, 2023

@alkino @pramodk

ci/gitlab/bbpgitlab.epfl.ch — Pipeline failed on GitLab

Excellent :) Seems like there is an excellent chance now that this result is the same as the result with #2470 . I For greater confidence I wonder if this and #2470 are the same for nrn-modeldb-ci

@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2023

NEURON ModelDB CI: launching for 8330f7a via its drop url

@pramodk
Copy link
Member

pramodk commented Sep 8, 2023

@nrnhines, as discussed yesterday, we will check the results!

By the way, thinking out loud: I wonder if we should merge this change into neuron master (before merging Eigen PR #2470). If even in the future one needs to verify the difference is due to Eigen or not, having this commit might be helpful? 🤔

@nrnhines
Copy link
Member Author

nrnhines commented Sep 8, 2023

@nrnhines, as discussed yesterday, we will check the results!

By the way, thinking out loud: I wonder if we should merge this change into neuron master (before merging Eigen PR #2470). If even in the future one needs to verify the difference is due to Eigen or not, having this commit might be helpful? 🤔

I think that is a good idea. That will better focus the mescach to eigen change in terms of a direct replacement. And it will be interesting to see how many modeldb model results differ because of this change.

@pramodk
Copy link
Member

pramodk commented Sep 8, 2023

Hello @nrnhines and @alkino,

A good and bad news! (as we were wondering if the gitlab CI i.e. BBP models would produce same results between this PR and #2470)

I took the CI directory from this PR: https://bbpgitlab.epfl.ch/hpc/sim/blueconfigs/-/jobs/890315
and the CI directory from #2470 : https://bbpgitlab.epfl.ch/hpc/sim/blueconfigs/-/jobs/889194

and then compared all out.dat generated in corresponding jobs of all BBP models:

job1=P150872
job2=P150688

#CI sim dir
job_dir_prefix=/gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim/

# all spike outputs from sim
spike_files=`find $job_dir_prefix/$job1/hpc/sim/blueconfigs -name out.dat`

for job1_spike_file in $spike_files
do
    # corresponding spike files from job2
    job2_spike_file=${job1_spike_file/$job1/$job2}

    change=$(diff $job1_spike_file $job2_spike_file)
    if [ "$change" != "" ]
    then
        echo -e "DIFF:: $job1_spike_file is different one from $job2 \n $change"
    else
        echo -e "SAME:: Results from $job1_spike_file is the same as one from $job2"
    fi
    echo -e "------------------------------------------\n\n"
done

and this produces following output:

kumbhar@bbpv2:~/tmp$ bash compare.sh

SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-v5-multisplit/output_multisplit_9159e30f/out.dat is the same as one from P150688
------------------------------------------


SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-v5-multisplit/output_9159e30f/out.dat is the same as one from P150688
------------------------------------------


SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/sonataconf-quick-hip-multipopulation/output_coreneuron_sonataconf_lb_55aac866/out.dat is the same as one from P150688
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-hip-delayconn/output_coreneuron_55aac866/out.dat is different one from P150688
 8c8
< 29.275	85732
---
> 29.3	85732
17c17
< 109.875	85732
---
> 109.9	85732
19c19
< 188.75	85732
---
> 188.8	85732
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-hip-delayconn/output_55aac866/out.dat is different one from P150688
 10,12c10,12
< 29.275	85732
< 109.875	85732
< 188.750	85732
---
> 29.300	85732
> 109.900	85732
> 188.800	85732
------------------------------------------


SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/multiscale/output_df0adf30/out.dat is the same as one from P150688
------------------------------------------


SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/sonataconf-quick-thalamus/save/out.dat is the same as one from P150688
------------------------------------------


SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/sonataconf-quick-thalamus/output_coreneuron_save_restore_2c64924b/out.dat is the same as one from P150688
------------------------------------------


SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-mousify-sonata/output_7f86b7b9/out.dat is the same as one from P150688
------------------------------------------


SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/thalamus/output_2c64924b/out.dat is the same as one from P150688
------------------------------------------


SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/thalamus/output_coreneuron_2c64924b/out.dat is the same as one from P150688
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/scx-1k-v5-newparams/output_coreneuron_9159e30f/out.dat is different one from P150688
 243c243
< 28.175	24
---
> 28.15	24
489c489
< 34.2	240
---
> 34.15	240
610a611
> 37.8	154
614d614
< 37.85	154
628c628
< 38.975	40
---
> 38.95	40
635a636
> 39.5	217
637d637
< 39.6	217
640c640
< 39.775	662
---
> 39.75	662
845d844
< 59.9	517
846a846
> 59.925	517
1011d1010
< 71.175	879
1013a1013
> 71.2	879
1132d1131
< 78.275	715
1133a1133
> 78.325	715
1329c1329
< 95.875	466
---
> 95.9	466
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/scx-1k-v5-newparams/output_9159e30f/out.dat is different one from P150688
 11c11
< 39.600	217
---
> 39.500	217
190c190
< 37.850	154
---
> 37.800	154
255c255
< 59.900	517
---
> 59.925	517
273c273
< 39.775	662
---
> 39.750	662
298c298
< 71.175	879
---
> 71.200	879
451c451
< 28.175	24
---
> 28.150	24
455c455
< 34.200	240
---
> 34.150	240
645c645
< 95.875	466
---
> 95.900	466
749c749
< 38.975	40
---
> 38.950	40
1249c1249
< 78.275	715
---
> 78.325	715
------------------------------------------


SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-hip-multipopulation/output_55aac866/out.dat is the same as one from P150688
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-v5-gaps/output_9159e30f/out.dat is different one from P150688
 4c4
< 37.275	90651
---
> 37.375	90651
61c61
< 37.200	84140
---
> 37.225	84140
80c80
< 31.600	87793
---
> 31.625	87793
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/sscx-v7-plasticity/output_b4009e8c/out.dat is different one from P150688
 45d44
< 611.400	3431136
53c52
< 88.150	3449494
---
> 660.100	3438985
61c60
< 660.100	3438985
---
> 88.150	3449494
65a65
> 611.400	3431136
91c91
< 214.350	3449292
---
> 760.050	3449487
93d92
< 812.350	3449292
97,99d95
< 760.050	3449487
< 76.800	3446685
< 447.450	3437041
101a98
> 560.425	3446426
102a100,101
> 447.450	3437041
> 214.350	3449292
105c104,105
< 560.425	3446426
---
> 812.350	3449292
> 76.800	3446685
------------------------------------------


SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/sscx-v7-plasticity/output_coreneuron_b4009e8c/out.dat is the same as one from P150688
------------------------------------------


SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-scx-multi-circuit/output_coreneuron_9159e30f/out.dat is the same as one from P150688
------------------------------------------


SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-scx-multi-circuit/output_9159e30f/out.dat is the same as one from P150688
------------------------------------------


SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-v6/output_quick_v6_fasthoc_9159e30f/out.dat is the same as one from P150688
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-v6/output_9159e30f/out.dat is different one from P150688
 43c43
< 4.125	87
---
> 4.075	87
45c45
< 39.075	87
---
> 43.050	87
86c86
< 16.375	271
---
> 16.350	271
------------------------------------------


SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-1k-v5-nodesets/output_9159e30f/out.dat is the same as one from P150688
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-hip-projSeed2/output_55aac866/out.dat is different one from P150688
 26c26
< 32.425	6410
---
> 32.450	6410
------------------------------------------


SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/sonataconf-quick-scx-multi-circuit/output_coreneuron_sonataconf_9159e30f/out.dat is the same as one from P150688
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/scx-1k-v5/output_scx_1k_v5_fasthoc_hocify_9159e30f/out.dat is different one from P150688
 197c197
< 15.400	80
---
> 15.425	80
245c245
< 12.300	154
---
> 12.275	154
1222c1222
< 9.400	49
---
> 9.425	49
------------------------------------------


SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/scx-1k-v5/output_lb_subtarget_py_9159e30f/out.dat is the same as one from P150688
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/scx-1k-v5/output_9159e30f/out.dat is different one from P150688
 197c197
< 15.400	80
---
> 15.425	80
245c245
< 12.300	154
---
> 12.275	154
1222c1222
< 9.400	49
---
> 9.425	49
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-v5-plasticity/output_coreneuron_delmodeldata_b4009e8c/out.dat is different one from P150688
 191c191
< 34.65	89628
---
> 34.625	89628
210c210
< 37.35	64862
---
> 37.325	64862
215c215
< 37.65	63259
---
> 37.625	63259
222c222
< 38.425	93551
---
> 38.45	93551
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-v5-plasticity/output_syndelayoverride_b4009e8c/out.dat is different one from P150688
 71c71
< 37.650	63259
---
> 37.625	63259
116c116
< 34.650	89628
---
> 34.625	89628
178c178
< 37.350	64862
---
> 37.325	64862
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-v5-plasticity/output_b4009e8c/out.dat is different one from P150688
 21c21
< 38.425	93551
---
> 38.450	93551
73c73
< 37.650	63259
---
> 37.625	63259
116c116
< 34.650	89628
---
> 34.625	89628
178c178
< 37.350	64862
---
> 37.325	64862
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-v5-plasticity/output_shm_coreneuron_b4009e8c/out.dat is different one from P150688
 191c191
< 34.65	89628
---
> 34.625	89628
210c210
< 37.35	64862
---
> 37.325	64862
215c215
< 37.65	63259
---
> 37.625	63259
222c222
< 38.425	93551
---
> 38.45	93551
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-v5-plasticity/output_multicycle_coreneuron_b4009e8c_bc/out.dat is different one from P150688
 191c191
< 34.65	89628
---
> 34.625	89628
210c210
< 37.35	64862
---
> 37.325	64862
215c215
< 37.65	63259
---
> 37.625	63259
222c222
< 38.425	93551
---
> 38.45	93551
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-v5-plasticity/output_manyreplay_b4009e8c/out.dat is different one from P150688
 191c191
< 34.65	89628
---
> 34.625	89628
210c210
< 37.35	64862
---
> 37.325	64862
215c215
< 37.65	63259
---
> 37.625	63259
222c222
< 38.425	93551
---
> 38.45	93551
------------------------------------------


diff: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150688/hpc/sim/blueconfigs/quick-v5-plasticity/output_coreneuron_model_building_py_b4009e8c/out.dat: No such file or directory
SAME:: Results from /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-v5-plasticity/output_coreneuron_model_building_py_b4009e8c/out.dat is the same as one from P150688
------------------------------------------


DIFF:: /gpfs/bbp.cscs.ch/ssd/gitlab_map_jobs/bbpcihpcsim//P150872/hpc/sim/blueconfigs/quick-v5-plasticity/output_coreneuron_model_building_auto_py_b4009e8c/out.dat is different one from P150688
 190c190
< 34.65	89628
---
> 34.625	89628
209c209
< 37.35	64862
---
> 37.325	64862
214c214
< 37.65	63259
---
> 37.625	63259
221c221
< 38.425	93551
---
> 38.45	93551
------------------------------------------

i.e. there are multiple models where there is difference is result between in this #2491 and #2470.

@nrnhines
Copy link
Member Author

nrnhines commented Sep 8, 2023

@pramodk

good and bad news!

Well. That's disappointing. Now we have to focus in more detail on a few of the models that differ. We'll have to discuss a bit so I can observe the situation with you. I guess it would be good to load single cells and compare the soma 3-d points. e.g. gid 85732. Or use the pc.prcellstate method.

@nrnhines
Copy link
Member Author

@pramodk

good and bad news!

This is too crazy to be the case, but in the old days, I seem to recall that neurodamus had its own version of Import3D. Surely that can't be relevant now?

@pramodk
Copy link
Member

pramodk commented Sep 11, 2023

neurodamus is now open source and it’s under the repo: https://github.com/BlueBrain/neurodamus

as far as I see, there is no separate import3d.

@alkino
Copy link
Member

alkino commented Sep 11, 2023

What about NeuroLucida_import3d?

@nrnhines
Copy link
Member Author

What about NeuroLucida_import3d?

That would not matter as this change is only in import3d_gui.hoc

@azure-pipelines
Copy link

✔️ ce2c143 -> Azure artifacts URL

@bbpbuildbot

This comment has been minimized.

@github-actions
Copy link
Contributor

NEURON ModelDB CI: launching for ce2c143 via its drop url

@pramodk
Copy link
Member

pramodk commented Sep 18, 2023

Here is a summary from today's discussion:

  • the change to share/lib/hoc/import3d/import3d_gui.hoc was:
diff --git a/share/lib/hoc/import3d/import3d_gui.hoc b/share/lib/hoc/import3d/import3d_gui.hoc
index fb39733..936677e 100755
--- a/share/lib/hoc/import3d/import3d_gui.hoc
+++ b/share/lib/hoc/import3d/import3d_gui.hoc
@@ -1224,7 +1224,15 @@ proc contour2centroid() {local i, j, imax, imin, ok  localobj mean, pts, d, max,
                        m.x[j][i] = m.x[i][j]
                }
        }
-       tobj = m.symmeig(m)
+
+    printf("----raw matrix----")
+    m.printf("%.16f ")
+    tobj = m.symmeig(m)
+    printf("----eigen vector-----")
+    m.printf("%.16f ")
+    printf("----eigen values-----")
+    tobj.printf("%.16f ")
  • Change to py-neurodamus was:
diff --git a/neurodamus/node.py b/neurodamus/node.py
index f283179..2a7233e 100644
--- a/neurodamus/node.py
+++ b/neurodamus/node.py
@@ -1461,6 +1461,11 @@ class Node:
         # design relatively to the original version where the report checkpoint would not happen
         # before the checkpoint timeout (25ms default). However there shouldn't be almost any
         # performance penalty since the simulation is already halted between events.
+        logging.info("---------------PRINTING 3D POINTS FOR A GID=37-----------")
+
+        s = self._pc.gid2cell(37).soma[0]
+        for i in range(s.n3d()):
+            print(i, s.x3d(i), s.y3d(i), s.z3d(i), s.diam3d(i))

Here is what we wanted to check:

Here is what we did:

  • Considering the failed Gitlab CI test, we choose a circuit that is small enough but also produces differences in spike results.
    ** we used scx-1k-v5 BlueConfig as it's small enough
    ** we used a single cell target with gid 37

  • We started comparing this PR against the master. The spike was already different:

$ diff output/out.dat ../scx-1k-v5-soma/output/out.dat
2c2
< 17.275	37
---
> 17.250	37

anyway, we were not expecting the same results between this PR and master.

  • The next interesting test was this PR against Replace Meschach by eigen #2470. We wanted to see why the results are different.
    ** First when compared the 3d points, we saw following i.e. only 1st 3 points were different. There positions were identical but the areas were different:
image
** we then looked at the matrix that was constructed and Eigen values/vectors that are calculated by meschach vs Eigen:
image
     here, we can see that the original matrix is the same but the Eigen values/vectors that are calculated are slightly different if we print the vectors with additional precision.
  • we then copied the corresponding morphology file /gpfs/bbp.cscs.ch/project/proj12/jenkins/cellular/circuit-1k/morphologies/ascii/dend-C280998A-P3_axon-C190898A-P3.asc and looked at it via nrngui's Tools->Miscellaneous->Import 3D tool then we inspected:
image

I don't understand all aspects but Michael has explained some of the details and the recording is available here (if someone is interested to see this in action).

Here is a side by side comparison of master vs this PR vs #2470:

image

IIUC, @nrnhines was OK in principle to merge #2470 after checking output of ModelDB CI.

@nrnhines:

Run diffreports2html ${nrn1_ver}.json ${nrn2_ver}.json
[18](https://github.com/neuronsimulator/nrn/actions/runs/6224454146/job/16892704222?pr=2491#step:12:19)
Writing /home/runner/work/nrn/nrn/9.0a-41-gcc1af3c27-vs-9.0a-37-ge742cdfc4.html ...
[19](https://github.com/neuronsimulator/nrn/actions/runs/6224454146/job/16892704222?pr=2491#step:12:20)
Done.
[20](https://github.com/neuronsimulator/nrn/actions/runs/6224454146/job/16892704222?pr=2491#step:12:21)
Writing /home/runner/work/nrn/nrn/runtimes-9.0a-41-gcc1af3c27-vs-9.0a-37-ge742cdfc4.html ...
[21](https://github.com/neuronsimulator/nrn/actions/runs/6224454146/job/16892704222?pr=2491#step:12:22)
Done.
[22](https://github.com/neuronsimulator/nrn/actions/runs/6224454146/job/16892704222?pr=2491#step:12:23)
FAILURE: stdout diffs in {'180373'}

maybe you can take a quick look?

@nrnhines
Copy link
Member Author

nrnhines commented Sep 18, 2023

It is disturbing to me that in your last row of figures, the contour PR (middle) and eigen PR (right) have opposite thick green major axis lines. Since I have the neurolucida file, I can investigate this more closely. My intention is that the thick green line should indicate the orientation of the major axis and those should be the same for contour and eigen. This will likely mean a small modification to Import3d_GUI.hoc. Will finish this today.

@nrnhines
Copy link
Member Author

Using

$ cat ~/neuron/temp.py
from neuron import h, gui
h.load_file("import3d.hoc")
swc = h.Import3d_Neurolucida3()
swc.input("/home/hines/Downloads/dend-C280998A-P3_axon-C190898A-P3.asc")
g = h.Import3d_GUI(swc)
g.instantiate(None)

s = h.soma[0]
for i in range(s.n3d()):
    print(i, s.x3d(i), s.y3d(i), s.z3d(i), s.diam3d(i))

I observe that when eigen and contour pr's are modified to force major axis orientation to be the same as master ( i.e with the above neurolucida file, only eigen multiplies the major eigenvector by -1 and neither reverse the 3d points), that soma 3-d points are double precision identical (with x3d(0) starting around 9.47... and x3d(20) ending around -9.02...).

On the other had, when eigen and contour pr's are modified to force major axis orientation to accord with the heuristic that the major axis vector angle is between -45 and 135 degrees (largest x,y is positive), then both eigen and contour have a major axis with orientation opposite (using this asc file) to the master (only contour multiplies the major eigenvector by -1). And eigen and contour have the same order and double precision values for the 3d points (in this case, also, the code was changed so that in this case both contour and eigen reversed the calculated 3dpoints so they basically correspond with the ordering of master)
In this case the last 3 3d point diameters for master, contour, and eigen are

10.201505661010742   10.110976219177246   10.110976219177246
7.78463888168335     7.467154502868652    7.467154502868652 
4.391671657562256    4.232929706573486    4.232929706573486

From these observations, it seems worthwhile to experiment with abandoning the point reversal of 8330f7a and choosing a major axis eigenvector orientation heuristic (largest absolute x, y is negative). I.e. the above file will produce same 3-d soma point ordering and values for master, contour, and eigen. And I expect contour and eigen will be consistent for all tests.
If there turn out to be MORE differences between master and contour, then we can return to the heuristic that largest absolute x, y is positive.

@azure-pipelines
Copy link

✔️ 38f8dad -> Azure artifacts URL

@bbpbuildbot

This comment has been minimized.

@nrnhines
Copy link
Member Author

nrnhines commented Sep 19, 2023

@pramodk I see that bbpgitlab is failing and wonder if you can tell me how many and provide me with a sample .asc file from one of the failures for this most recent 38f8dad. From your long comment above for 8330f7a I count 18 SAME and 17 DIFF
I'd also be curious if every one of the failures also contains one or more lines of reorient major in the stdout which would mean contour chose a different orientation of the major axis from the master.

@pramodk
Copy link
Member

pramodk commented Sep 20, 2023

Just to update here: we look at the failing test in CI and provided corresponding ASCII morphology files to Michael:

/gpfs/bbp.cscs.ch/project/proj12/jenkins/cellular/circuit-1k/morphologies/ascii

@nrnhines is going to check if there is a "common heuristic" that can be used so that we get same results from master, this PR as well as Eigen PR.

@nrnhines
Copy link
Member Author

I analyzed the 60 asc morphology files from /gpfs/bbp.cscs.ch/project/proj12/jenkins/cellular/circuit-1k/morphologies/ascii using three heuristics.

Orient closest to the i + j unit vector direction
Orient closest to the -i - j unit vector direction
Orient closest to point 0 on the contour

For each, contour and eigen soma 3dpoints agree. But contour differs from master for 34, 26, and 28 asc files. With respect to contour and eigen, most often, reorientation of the major axis direction happens only for one and not the other, but there are a few files where reorientation happens for contour and eigen with the same asc file and sometimes contour and eigen accept the orientation produced by symmeig. I know of no way to predict the orientation generated by meschach from the computed eigenvalues and eigenvectors generated by eigen. That does not mean there isn't a simple way to do so.

@azure-pipelines
Copy link

✔️ 831a210 -> Azure artifacts URL

@nrnhines
Copy link
Member Author

I'm investigating one more heuristic based on the eigenvalue order and eigenvector orientation.

@nrnhines
Copy link
Member Author

Unfortunately, I see no way to use the eigenvalue,eigenvector information calculated by eigen to predict the major axis orientation
calculated from meshach. The qualitative information content of eigen calculations (order of eigenvalues and direction of the largest element of the associated eigenvector, often allows the decision to reorient the major axis to go either way.)

In the following analysis results for the 60 morphology files,
The first item of each tuple refers to the eigen order and orientation information.
0,1,2 refer to smallest, minor, major axis order and +,- refers to the orientation of the axis
The second item of each tuple is the set of corresponding master orientation of the major axis.
Since the same eigen patterns can be associated with both + and - master major axis orientations, one cannot decide which to choose.
The analysis
results for the 60 morphologies are

('1+0-2-', {'+', '-'})
('1+0+2+', {'-'})
('2+0-1+', {'+', '-'})
('1-0+2-', {'+', '-'})
('2-0+1+', {'+', '-'})
('2+0+1+', {'-'})
('2-1-0+', {'+'})
('2-0-1+', {'-'})
('1-0-2-', {'-'})
('1-0-2+', {'+', '-'})
('2+1+0+', {'+'})
('2-1+0+', {'+'})
('1+2-0+', {'-'})
('2+1-0+', {'-'})

@pramodk pramodk merged commit ba1cedc into master Oct 12, 2023
30 of 31 checks passed
@pramodk pramodk deleted the hines/contour-test branch October 12, 2023 20:32
pramodk pushed a commit that referenced this pull request Oct 12, 2023
* As part of the legacy code modernization, we are replacing old in-source
copy of ~20-year-old Meschach library with a modern alternative - Eigen!

* However, be aware that this change will inevitably impact results, particularly
if we expect the floating point/spike times identical results for simulations.

* In #2491, we did a detailed analysis of the root cause. The conclusion is that the
floating point differences can be safely ignored.

Co-authored-by: Michael Hines <[email protected]>
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.

4 participants