Adds support for Section target reports by sergiorg-hpc · Pull Request #419 · BlueBrain/CoreNeuron

sergiorg-hpc · 2020-11-11T11:09:13Z

This commit adds support to enable reports for both a Target Section, or a Target Compartment with a Section inside. The behaviour is the same as in NEURON, providing the middle value in the first case, while providing all the values in the second case with Compartment.

To summarize the changes, the solution extends the purpose of one boolean in the report.conf file to encode new types of targets, such as axon, axonComp (for Compartments), and so on. This guarantees file compatibility with previous versions of CoreNEURON. The new encoded type is retrieved as an enum class and used to determine the type of report.

More information, including instructions on how to request the reports, can be found in the ticket:
https://bbpteam.epfl.ch/project/issues/browse/CNEUR-362?focusedCommentId=131257&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-131257

CI_BRANCHES:NEURON_BRANCH=master

bbpbuildbot · 2020-11-11T11:09:16Z

Can one of the admins verify this patch?

bbpbuildbot · 2020-11-11T11:09:16Z

Can one of the admins verify this patch?

bbpbuildbot · 2020-11-11T11:09:16Z

Can one of the admins verify this patch?

pramodk · 2020-11-11T11:52:38Z

Can one of the admins verify this patch?

I added sergiorg-hpc username to CI plans so that we won't have above message and CI will trigger all build plans.

pramodk

some minor comments/feedback.

sergiorg-hpc · 2020-11-17T11:54:29Z

@jorblancoa and I have introduced several changes that addresses the suggestions of the review, thank you so much for the insight!

To clarify, we have tested again generating all the reports on a small test case, and compared the result with NEURON. This is just to confirm that the latest changes didn't introduce any errors.

sergiorg-hpc · 2020-11-17T13:17:13Z

Important: The CI is not passing because of the NEURON version issue (i.e., the sandbox/srivas/report_sections branch was updated recently to master). However, we relaunched the CI from the blueconfigs below and it works perfectly fine:

https://bbpcode.epfl.ch/ci/blue/organizations/jenkins/hpc.SimulationStack/detail/hpc.SimulationStack/2687/pipeline
https://bbpcode.epfl.ch/code/#/c/51354/

pramodk

2 minor comments otherwise LGTM

Co-authored-by: Pramod Kumbhar <pramod.kumbhar@epfl.ch>

Co-authored-by: Alexandru Săvulescu <46521150+alexsavulescu@users.noreply.github.com>

alexsavulescu

LGTM

Co-authored-by: Alexandru Săvulescu <46521150+alexsavulescu@users.noreply.github.com>

Co-authored-by: Alexandru Săvulescu <alexandru.savulescu@epfl.ch>

NEURON_BRANCH=master

iomaganaris · 2020-11-18T10:59:41Z

please retest

pramodk · 2020-11-18T21:40:35Z

So the conclusion is NEURON_BRANCH is somehow not working in this case?

iomaganaris · 2020-11-19T08:31:56Z

So the conclusion is NEURON_BRANCH is somehow not working in this case?

Yes, there is some issue with the multiline description and the way the NEURON_BRANCH is filtered in the CI. I opened a PR in the blueconfigs repo to address the issue. It should be finished today
This PR is also tested in https://bbpcode.epfl.ch/code/#/c/51354/ with the Simulation Stack

iomaganaris · 2020-11-19T08:57:19Z

please retest

sergiorg-hpc · 2020-11-19T10:58:25Z

please retest

sergiorg-hpc · 2020-11-19T11:44:08Z

Hi there!

Thanks to @iomaganaris' great work, we have the CI back on track and now it can be confirmed that the PR changes passes it. Nonetheless, the following PR is the only one that evaluates the support for Section targets inside CoreNEURON:
https://bbpcode.epfl.ch/code/#/c/51354

Is there any additional changes you would like to integrate, or can we merge this already? If so, can someone approve the PR, please?

sergiorg-hpc · 2020-11-19T14:54:17Z

Thank you everyone for the help and insight!

* Adds support for Axon, Dendrite, and Apical sections in reports * Prevents to generate an empty list when the section is empty * Adds support for Section in a Compartment * Introduces suggestions by code review in PR * Updates description of the enum Co-authored-by: Pramod Kumbhar <pramod.kumbhar@epfl.ch> * Adds an error when the Section type is unknown Co-authored-by: Alexandru Săvulescu <46521150+alexsavulescu@users.noreply.github.com> * Adds ref to the section_type_str Co-authored-by: Alexandru Săvulescu <46521150+alexsavulescu@users.noreply.github.com> * Adds an error when the target type is unknown * Switches abort() for nrn_abort() in the configuration parser Co-authored-by: Alexandru Săvulescu <46521150+alexsavulescu@users.noreply.github.com> * Switched assert() to nrn_assert() Co-authored-by: Alexandru Săvulescu <alexandru.savulescu@epfl.ch> * Extends the previous commit NEURON_BRANCH=master Co-authored-by: Pramod Kumbhar <pramod.s.kumbhar@gmail.com> Co-authored-by: Pramod Kumbhar <pramod.kumbhar@epfl.ch> Co-authored-by: Alexandru Săvulescu <46521150+alexsavulescu@users.noreply.github.com> Co-authored-by: Alexandru Săvulescu <alexandru.savulescu@epfl.ch> CoreNEURON Repo SHA: BlueBrain/CoreNeuron@9dfaabb

sergiorg-hpc added 3 commits November 4, 2020 17:19

Adds support for Axon, Dendrite, and Apical sections in reports

0dfd7df

Prevents to generate an empty list when the section is empty

7053c88

Adds support for Section in a Compartment

d4f1ee1

sergiorg-hpc requested review from alkino, jorblancoa and pramodk November 11, 2020 11:09

sergiorg-hpc self-assigned this Nov 11, 2020

pramodk closed this Nov 11, 2020

pramodk reopened this Nov 11, 2020

Merge branch 'master' into sandbox/srivas/report_sections

b68517c

alkino reviewed Nov 12, 2020

View reviewed changes

Comment thread coreneuron/io/reports/report_configuration_parser.cpp Outdated

Comment thread coreneuron/io/reports/report_handler.cpp Outdated

pramodk reviewed Nov 12, 2020

View reviewed changes

Comment thread coreneuron/io/reports/nrnreport.hpp Outdated

Comment thread coreneuron/io/reports/report_handler.cpp Outdated

Comment thread coreneuron/io/reports/report_handler.cpp

Comment thread coreneuron/io/reports/report_handler.cpp Outdated

Merge branch 'master' into sandbox/srivas/report_sections

0aeefae

alexsavulescu reviewed Nov 17, 2020

View reviewed changes

Introduces suggestions by code review in PR

eab96cd

alexsavulescu reviewed Nov 17, 2020

View reviewed changes

Comment thread coreneuron/io/reports/report_handler.cpp Outdated

alexsavulescu reviewed Nov 17, 2020

View reviewed changes

Comment thread coreneuron/io/reports/report_handler.cpp Outdated

pramodk reviewed Nov 17, 2020

View reviewed changes

Comment thread coreneuron/io/reports/report_configuration_parser.cpp Outdated

Comment thread coreneuron/io/reports/report_handler.cpp Outdated

Comment thread coreneuron/io/reports/report_configuration_parser.cpp Outdated

Comment thread coreneuron/io/reports/report_handler.cpp

sergiorg-hpc and others added 4 commits November 17, 2020 16:48

Updates description of the enum

5e06c12

Co-authored-by: Pramod Kumbhar <pramod.kumbhar@epfl.ch>

Adds an error when the Section type is unknown

6c5cb52

Co-authored-by: Alexandru Săvulescu <46521150+alexsavulescu@users.noreply.github.com>

Adds ref to the section_type_str

0abbae3

Co-authored-by: Alexandru Săvulescu <46521150+alexsavulescu@users.noreply.github.com>

Adds an error when the target type is unknown

3b4c218

alexsavulescu reviewed Nov 17, 2020

View reviewed changes

Comment thread coreneuron/io/reports/report_configuration_parser.cpp Outdated

alexsavulescu reviewed Nov 17, 2020

View reviewed changes

Comment thread coreneuron/io/reports/report_configuration_parser.cpp

alexsavulescu reviewed Nov 17, 2020

View reviewed changes

Switches abort() for nrn_abort() in the configuration parser

fc9b001

Co-authored-by: Alexandru Săvulescu <46521150+alexsavulescu@users.noreply.github.com>

alexsavulescu reviewed Nov 18, 2020

View reviewed changes

Comment thread coreneuron/io/reports/report_handler.cpp Outdated

sergiorg-hpc and others added 2 commits November 18, 2020 09:28

Switched assert() to nrn_assert()

d4680dc

Co-authored-by: Alexandru Săvulescu <alexandru.savulescu@epfl.ch>

Extends the previous commit

f605030

NEURON_BRANCH=master

alexsavulescu approved these changes Nov 19, 2020

View reviewed changes

alexsavulescu merged commit 9dfaabb into master Nov 19, 2020

sergiorg-hpc deleted the sandbox/srivas/report_sections branch November 19, 2020 15:01

Uh oh!

Conversation

sergiorg-hpc commented Nov 11, 2020 • edited by iomaganaris Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bbpbuildbot commented Nov 11, 2020

Uh oh!

bbpbuildbot commented Nov 11, 2020

Uh oh!

bbpbuildbot commented Nov 11, 2020

Uh oh!

pramodk commented Nov 11, 2020

Uh oh!

Uh oh!

Uh oh!

pramodk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sergiorg-hpc commented Nov 17, 2020

Uh oh!

sergiorg-hpc commented Nov 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pramodk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexsavulescu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

iomaganaris commented Nov 18, 2020

Uh oh!

pramodk commented Nov 18, 2020

Uh oh!

iomaganaris commented Nov 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iomaganaris commented Nov 19, 2020

Uh oh!

sergiorg-hpc commented Nov 19, 2020

Uh oh!

sergiorg-hpc commented Nov 19, 2020

Uh oh!

sergiorg-hpc commented Nov 19, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

sergiorg-hpc commented Nov 11, 2020 •

edited by iomaganaris

Loading

sergiorg-hpc commented Nov 17, 2020 •

edited

Loading

iomaganaris commented Nov 19, 2020 •

edited

Loading