Arbor internal syntax parser is coded in bluepyopt

anilbey commented 2 months ago

This module for parsing Arbor syntax is part of BluePyOpt. Changes in newer versions of Arbor cannot be reflected in this module because it is integrated within BluePyOpt. To ensure BluePyOpt remains up-to-date with the latest Arbor versions, this module should be imported directly from Arbor.

https://github.com/BlueBrain/BluePyOpt/blob/aff67f82758387500e47a377fe5b0c689b4a5a24/bluepyopt/ephys/parameterscalers/acc_iexpr.py#L27

thorstenhater commented 2 months ago

Hi @anilbey,

I found this by accident and as the resident Arbor person I thought I'd offer a hand. @lukasgd has made the BluePyOpt to Arbor interface happen, so maybe he has thoughts, too. The problem is, as time passes, APIs and formats will evolve and in this particular case, both did change. The ACC file version is now '0.9.0', matching the Arbor version and we tweaked the way inhomogeneous parameters are serialized and represented at the API layer.

The API, including the parsers, are public. You'll find them by looking for load_component. That said, using this method on the old files will result in an Exception due to version mismatch. If you want help updating this, just ping us.

anilbey commented 2 months ago

Hi @thorstenhater thanks for offering your help. I will not be around (I am leaving soon) but the maintainers of BPO can help. cc @AurelienJaquier @ilkilic @darshanmandge

lukasgd commented 2 months ago

Hi @anilbey and @thorstenhater,

Thanks for bringing this up in an issue. I think there's a bit a misunderstanding with the purpose of the acc_iexpr module. It's not an Arbor syntax parser, but it's actually generating ACC expressions from BluePyOpt's internal representation of inhomogeneous parameters. The main interface is the method generate_acc_scale_iexpr in that module.

This functionality is necessary so that BluePyOpt can export models that can be run with Arbor and, therefore, also support Arbor as an optimization backend. When writing the ACC exporting infrastructure, I tried to stay as consistent as possible with the HOC/Neuron code, so that BluePyOpt could be used as a frontend to these different simulator backends. As such, it needs to be able to export model state to drive optimizations and store results.

Having said this, I see your concern of having the exported format getting out of date with what Arbor expects. At the moment the tox tests on my system (Python 3.10) are still succeeding as I installed Arbor as a dependency through pip (v0.9.0), but the current main branch should introduce an exception as Thorsten says above.

I believe this should already be visible in CI currently, since e.g. simplecell_arbor.ipynb is executed as part of it. The fix may then be done in the templates at bluepyopt/ephys/templates/acc. To regenerate the test data, you can use the generate_acc.py scripts in the examples directory (l5pc, simplecell and expsyn).

Back when Arbor support was added, we did not add L5PC_arbor.py to CI (instead focused on a consistency check between Arbor and Neuron with @wvangeit), so I it may be true that iexprs are not parsed by Arbor in any test currently. If you'd like to do that, e.g. for the expression at https://github.com/BlueBrain/BluePyOpt/blob/master/bluepyopt/tests/test_ephys/test_parameterscalers.py#L106-L109, you could use something inspired by this code

import pathlib
import tempfile
import arbor

# iexpr from bluepyopt/tests/test_ephys/test_parameterscalers.py
iexpr = '(sub (scalar 0.62109375) ' \
        '(mul (log (pi) ) ' \
        '(exp (div (distance (region "soma")) ' \
        '(scalar 0.421875) ) ) ) )'

# modified decor as in bluepyopt/tests/test_ephys/testdata/acc/simplecell/simple_cell_decor.acc
simple_cell_decor_with_iexpr = \
    '(arbor-component\n' \
    '  (meta-data (version "0.1-dev"))\n' \
    '  (decor\n' \
    '    (paint (region "soma") (membrane-capacitance 0.01))\n' \
    '    (paint (region "soma") (scaled-mechanism (density (mechanism "default::hh" ' \
   f'("gnabar" 0.10299326453483033) ("gkbar" 0.027124836082684685))) ("gkbar" {iexpr})))))'

with tempfile.TemporaryDirectory() as test_dir:
        decor_filename = pathlib.Path(test_dir).joinpath("decor.acc")
        with open(decor_filename, "w") as f:
            f.write(simple_cell_decor_with_iexpr)
        test_decor = arbor.load_component(decor_filename).component
        print(f'decor {decor_filename} with\n'
              f'  defaults: {test_decor.defaults()}\n'
              f'  paintings: {test_decor.paintings()}\n'
              f'  placements: {test_decor.placements()}')

which currently still works on my system.

Probably more important would be to add additional arbor.load_component statements like in https://github.com/BlueBrain/BluePyOpt/blob/4fcf47a075ecbe2aa967040d297d0131339a75e3/bluepyopt/tests/test_ephys/test_create_acc.py#L385 to that file (this one should also trigger a version mismatch exception btw). If you search e.g. for l5pc in that file, you'll see that the test checks for string equality of the label_dict and decor ACC files. It think it wouldn't harm to add more checks below, such as an arbor.load_component on these files. This would then guarantee that all three exported models - simplecell, l5pc and expsyn - are parsable in Arbor, so a faulty iexpr in l5pc would be caught in CI.

thorstenhater commented 2 months ago

We also add a regular run of a simple BPO script to Arbor's CI to ensure we do not break you code.

AurelienJaquier commented 3 weeks ago

Hi @thorstenhater @lukasgd , it looks like since the new 0.10 arbor release, our tests here on BluePyOpt do not pass anymore. Is this related to this issue or is it something new? I tried using generate_acc.py in the l5pc example with an up-to-date arbor version, but it does not bring any changes with respect to the files we are already using in the tests. Am I misunderstanding something? Could you guys make BluePyOpt and its tests compatible with latest arbor please? Here is a failing test for reference: https://github.com/BlueBrain/BluePyOpt/actions/runs/10336590694/job/28612509119

thorstenhater commented 3 weeks ago

Hi @AurelienJaquier,

it seems your tests are using a fixed .acc file, however, I am not familiar with the details. The format was changed due to an update to the handling of inhomogeneous parameters, thus the format metadata was changed to 0.9.0-dev (the Arbor version of the update).

Best, T

wvangeit commented 3 weeks ago

Is there maybe an automatic converter from the old format to the newer?

thorstenhater commented 3 weeks ago

No, currently not. A

eous parameters are serialized and represented at the API layer.

The API, including the parsers, are public. You'll find them by looking for load_component. That said, using this method on the old files will result in an Exception due to version mismatch. If you want help updating this, just ping us.

Btw, this is precisely what I warned about on 20th of June.

AurelienJaquier commented 2 weeks ago

I opened a PR to make BluePyOpt compatible with latest arbor and make the tests pass: #498 Is it ok if I ask to to review it in order to have a look from the arbor team @thorstenhater ?

lukasgd commented 2 weeks ago

Hi @AurelienJaquier, thanks for your work!

I'm just having a brief look (would have more time in a month). I think you've correctly identified the acc/*_template.jinja2 files that need to be modified. They're used by all model exporters (also in examples), which is why the generate_acc.py failed for you before. For the specific changes, including the (scalar 1.0) multiplier for default and painted parameters, maybe Thorsten can comment.

There are some other changes to morphology (only for SWC) and protocols (stimulus envelope, units) that I'm not sure if they are required for compatibility. Did something not work there before?

Also, to return to the earlier point about checking compatibility in tests - have you considered adding arbor.load_component to tests as suggested at the end of this comment?

AurelienJaquier commented 2 weeks ago

Hi @lukasgd thank you for checking my PR. Indeed, I had to change the morphology loading and adding units because without that the tests were not passing:

changes to morphology: for some reason, some morphology format return a loaded_moprhology instead of a morphology, which has not the same attributes, so I had to make sure they are all of type morphology to be able to use them consistently
changes to units: it looks like a lot of places in arbor now do not accept numbers anymore, but require a value associated with a arbor unit, so I had to update those.

About the load_component tests, I have added the one depending on iexpr, as you described above. Could you have a look and tell me if it looks ok to you? I would like to be sure I am doing it correctly before doing the other ones (singlecell, l5pc and expsyn)

thorstenhater commented 2 weeks ago

Hi @AurelienJaquier,

we made some changes to our I/O support. loaded_morphology (naming is hard) is essentially this

{
  morphology  # the actual morphology
  segment_tree # raw segment information
  label_dict # table of labelled locations on the morphology, if any      
  metadata # loader specific information
}

Thus, all loaders share one interface and do more for the user.

Units is also a new feature in v0.10.0, requiring the explicit use of a unit at the interface level.

Since BluePyOpt+Arbor already depends on Arbor, is using something like this an option?

import arbor as A

cell = A.cable_cell(morphology, decor, labels)
A.write_component(cell)

This will use our internal serialization to .acc making these templates redundant.

lukasgd commented 2 weeks ago

W.r.t. using Arbor for serialization vs. templates, see https://github.com/BlueBrain/BluePyOpt/pull/498#discussion_r1727122992.

For the iexpr loading test @AurelienJaquier, I think you could already stop at the line test_decor = arbor.load_component(decor_filename).component. Invoking arbor.load_component should make sure that the expressions in the file are parsable. Checking the semantic correctness of this call should be covered by Arbor's test suite already.

To check the examples, you could add an arbor.load_component call in the check_acc_dir function for non-JSON files. I.e. after this line: https://github.com/BlueBrain/BluePyOpt/blob/4fcf47a075ecbe2aa967040d297d0131339a75e3/bluepyopt/tests/test_ephys/test_create_acc.py#L667

BlueBrain / BluePyOpt

Arbor internal syntax parser is coded in bluepyopt #494