nrn icon indicating copy to clipboard operation
nrn copied to clipboard

Callbacks for reading mapping information from memory

Open WeinaJi opened this issue 2 years ago • 35 comments

neurodamus branch https://github.com/BlueBrain/neurodamus/tree/weji/direct_mod to launch coreneuron simulation with in memory mode.

WeinaJi avatar Sep 28 '23 07:09 WeinaJi

✔️ 3d6b9e58fc22aba2057b3b434fc47f0ad0558e28 -> Azure artifacts URL

azure-pipelines[bot] avatar Sep 28 '23 08:09 azure-pipelines[bot]

Codecov Report

Attention: Patch coverage is 88.09524% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 67.14%. Comparing base (e3e8625) to head (a3b93c2).

Files Patch % Lines
src/coreneuron/io/phase3.cpp 86.11% 5 Missing :warning:
src/coreneuron/io/core2nrn_data_return.cpp 0.00% 2 Missing :warning:
src/nrniv/nrncore_write/io/nrncore_io.cpp 88.23% 2 Missing :warning:
src/coreneuron/io/nrn_filehandler.hpp 75.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2555      +/-   ##
==========================================
+ Coverage   66.92%   67.14%   +0.22%     
==========================================
  Files         560      561       +1     
  Lines      104091   104145      +54     
==========================================
+ Hits        69658    69932     +274     
+ Misses      34433    34213     -220     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Sep 28 '23 08:09 codecov[bot]

✔️ 96276d086b377479db6eaec47b194f1119f8f3f5 -> Azure artifacts URL

azure-pipelines[bot] avatar Sep 28 '23 15:09 azure-pipelines[bot]

✔️ c9060889e21dffa9221ddc666dd4b416fcc41e7c -> Azure artifacts URL

azure-pipelines[bot] avatar Dec 04 '23 08:12 azure-pipelines[bot]

✔️ d9cba6b118f8f39fcf74c1ce30fdbdff87078000 -> Azure artifacts URL

azure-pipelines[bot] avatar Dec 04 '23 16:12 azure-pipelines[bot]

✔️ 86f969e0e07d41b3eff36385059a5c1c9898bad3 -> Azure artifacts URL

azure-pipelines[bot] avatar Dec 08 '23 21:12 azure-pipelines[bot]

✔️ 1b0a9a8891af37b1d0b6d14e5922e06d6f11403a -> Azure artifacts URL

azure-pipelines[bot] avatar Dec 12 '23 13:12 azure-pipelines[bot]

✔️ c54bf23721bf16d621a623a4e754d0960b66009f -> Azure artifacts URL

azure-pipelines[bot] avatar Dec 12 '23 14:12 azure-pipelines[bot]

Looks good!! Thanks! Just one extra comment, is it possible to create some unit tests? At least for the nrncore_callbacks (nrnthread_dat3_cell_count, nrnthread_dat3_cellmapping, etc.) I guess for read_phase3() would be a bit more complicated...although i did something similar for LFP populating the cellmapping, mapinfo etc

jorblancoa avatar Dec 14 '23 14:12 jorblancoa

✔️ cdce92c777cd2cfc064403c8ed13091e8bff4f03 -> Azure artifacts URL

azure-pipelines[bot] avatar Jan 07 '24 18:01 azure-pipelines[bot]

✔️ 3057bbdf9532d275f43f27c9a27f76059e95a4c8 -> Azure artifacts URL

azure-pipelines[bot] avatar Jan 12 '24 15:01 azure-pipelines[bot]

✔️ e939e3be89687531580a6c4b4e555eb230305d49 -> Azure artifacts URL

azure-pipelines[bot] avatar Jan 12 '24 16:01 azure-pipelines[bot]

✔️ 88a9a707b7599c5b32cf3ec0b705225c824bb998 -> Azure artifacts URL

azure-pipelines[bot] avatar Jan 13 '24 16:01 azure-pipelines[bot]

✔️ c8c5f44e62966ea008b68f718b6609106256f0f7 -> Azure artifacts URL

azure-pipelines[bot] avatar Jan 13 '24 18:01 azure-pipelines[bot]

✔️ 607c15ee5600269a51cbd2745aee78e359b475db -> Azure artifacts URL

azure-pipelines[bot] avatar Jan 14 '24 18:01 azure-pipelines[bot]

Looks good!! Thanks! Just one extra comment, is it possible to create some unit tests? At least for the nrncore_callbacks (nrnthread_dat3_cell_count, nrnthread_dat3_cellmapping, etc.) I guess for read_phase3() would be a bit more complicated...although i did something similar for LFP populating the cellmapping, mapinfo etc

I am trying to add a test but I am not sure where to locate the test. Since we have the callbacks map like

static core2nrn_callback_t cnbs[] = {
    ...
    {"nrn2core_get_dat3_cell_count_", (CNB) nrnthread_dat3_cell_count},
    ...
}

Shall I test function nrn2core_get_dat3_cell_count_ in coreneuron? If so, do I need to call first https://github.com/neuronsimulator/nrn/blob/45cdea7e74b0d695d1597589b47353d728082454/src/nrniv/nrncore_write.cpp#L305 ?

Or shall I test function nrnthread_dat3_cell_count? Is that in the scope of neuron ?

Maybe @pramodk @iomaganaris @jorblancoa have more insights?

WeinaJi avatar Jan 15 '24 15:01 WeinaJi

Copying comments from internal slack discussion:

If your test works for neurodamus tests and if you want a similar test in neuron then I wonder if we should just use ringtest which already in NEURON:

* ringtest has this  setup_nrnbbcore_register_mapping() : https://github.com/neuronsimulator/ringtest/blob/344bd13f8fb94d69e83b6645350f723079b3a21d/ringtest.py#L179

* and that registers section/segment mapping like neurodamus: https://github.com/neuronsimulator/ringtest/blob/344bd13f8fb94d69e83b6645350f723079b3a21d/commonutils.py#L170

We can add a separate CLI flag instead of dumpmodel but I wonder if we can do the following:
 * take one of the existing ringtest for coreneuron
  * in one of the online ringtest, we enable extra CLI flag --report-mapping and that will trigger your part of the code

pramodk avatar Jan 27 '24 08:01 pramodk

✔️ e9957cd20596d210169ccdd384e8311b2c93430b -> Azure artifacts URL

azure-pipelines[bot] avatar Feb 09 '24 07:02 azure-pipelines[bot]