De-duplicate variables

[thorstenhater]

No description provided.

[thorstenhater]

I am closing PR #34; this PR subsumes it completely and seems to not include the bug you noticed.

[llandsmeer]

Still get this:

thread 'main' panicked at 'internal error: entered unreachable code', src/expr.rs:492:38
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[llandsmeer]

The missing case is (Cmp::Gt, Cmp::Eq)

In general the following cases seem to be missing there:

(Cmp::Eq, Cmp::Gt)
(Cmp::Eq, Cmp::Lt)
(Cmp::Ge, Cmp::Ne)
(Cmp::Ge, Cmp::Gt)
(Cmp::Gt, Cmp::Eq)
(Cmp::Gt, Cmp::Ge)
(Cmp::Le, Cmp::Nq)
(Cmp::Le, Cmp::Lt)
(Cmp::Lt, Cmp::Le)
(Cmp::Lt, Cmp::Eq)
(Cmp::Ne, Cmp::Le)
(Cmp::Ne, Cmp::Ge)

[thorstenhater]

These should be the cases that we rewrite into some other form, right?
Example: x < y || x == y becomes x <= y and on. That raises two questions

1. Why doesn't my (same?) input trigger this?
2. Why doesn't the rewrite happen?

[thorstenhater]

Could you provide me an input that triggers the problem?

[thorstenhater]

So, there were some missing and honestly plain incorrect cases in there. Please double check again.

[llandsmeer]

Seems like I missed the github notifications. The input that triggered the problem was Hay et al 2011. The missing cases problem is solved now. With the new version, I get this:

  File "/home/llandsmeer/tmp/nmlcc35/hay/main.py", line 110
    raise \"oops\"

which looks like a quotation error. But raising a string is also not possible in Python.

Maybe:

raise NotImplementedError("Regular generators are not supported")

[llandsmeer]

If I fix this in main.py the resulting voltage trace is also completely wrong (settles to -85)

[thorstenhater]

Oh dear.

[llandsmeer]

The only differences in the mod files are things like

101,105c73
<     if (gates_h_forwardRate_x == 0) {
<       gates_h_forwardRate_r = 0.09000000357627869
<     } else {
<       gates_h_forwardRate_r = 0
<     }
---
>     gates_h_forwardRate_r = 0.09000000357627869

which I guess means that the previous case was wrong and that it's now fixed?

So I suspect it's the input generator that's broken

[thorstenhater]

No, this is an honest to god bug, we want to reduce redundant variables like

q10 = q^(T - T_0)
p10 = q^(T - T_0)

to

q10 = q^(T - T_0)
p10 = q10

What you observe is a unrelated and wrong.

[llandsmeer]

Here

    def event_generators(self, gid):
        res = []
        if gid in self.gid_to_inputs:
            for seg, frac, inp in self.gid_to_inputs[gid]:
                tag = f'(on-components {frac} (region \"{seg}\"))'
                if inp in self.poisson_generators:
                    syn, avg, wgt = self.poisson_generators[inp]
                    res.append(A.event_generator(f'syn_{syn}@seg_{seg}_frac_{frac}', wgt, A.poisson_schedule(0, avg, gid)))
                if inp in self.regular_generators:
                    raise 1
        return res

The tag variable is unused

[thorstenhater]

Fixed the Python template, it's now the same as the CXX template.

[llandsmeer]

    self.gid_to_cell = { int(k): v for k, v in data['gid_to_cell'].items() }
AttributeError: 'list' object has no attribute 'items'

gid_to_cell is a list and doesnt need the int(k):v thing

[llandsmeer]

Apart from that, Hay et al. works as expected again

[thorstenhater]

👍

[llandsmeer]

    self.gid_to_cell = data['gid_to_cell'].items()

Still need to remove the .items() call

[llandsmeer]

main.cxx also contains a lot of things like this:

        gid_to_cell = data[\"gid_to_cell\"];
        cell_to_morph = data[\"cell_to_morph\"];
        i_clamps = data[\"i_clamps\"];

which also looks like quotation errors

[llandsmeer]

Python code now works. For C++ via run.sh I still get:

CMake Error at CMakeLists.txt:6 (find_package):
  Could not find a configuration file for package "nlohmann_json" that is
  compatible with requested version "3.11.0".

  The following configuration files were considered but not accepted:

    /usr/lib/cmake/nlohmann_json/nlohmann_jsonConfig.cmake, version: 3.10.5
    /lib/cmake/nlohmann_json/nlohmann_jsonConfig.cmake, version: 3.10.5

[llandsmeer]

If I put find_package(nlohmann_json 3.10.0 REQUIRED) into the cmake thing

I get this error:

/home/llandsmeer/tmp/nmlcc35/tmpdir/main.cxx: In constructor ‘recipe::recipe(const std::filesystem::__cxx11::path&, const string&)’:
/home/llandsmeer/tmp/nmlcc35/tmpdir/main.cxx:59:41: error: ambiguous overload for ‘operator=’ (operand types are ‘std::vector<std::__cxx11::basic_string<char> >’ and ‘nlohmann::basic_json<>::value_type’ {aka ‘nlohmann::basic_json<>’})
   59 |         gid_to_cell = data["gid_to_cell"];

which might be related to the nlohman_json version. Still, 3.10 is the Ubuntu 22 LTS default

nlohmann-json3-dev/jammy,jammy,now 3.10.5-2 all [installed,automatic]
  JSON for Modern C++

and as such I think we should support it

[llandsmeer]

This does work:

        gid_to_cell = data["gid_to_cell"].get<decltype(gid_to_cell)>();

Now when I do this:

bash run.sh dat/network_L5bPyrCellHayEtAl2011.json

I get:

+ ./main dat/network_L5bPyrCellHayEtAl2011.json
terminate called after throwing an instance of 'nlohmann::detail::parse_error'
  what():  [json.exception.parse_error.101] parse error at line 1, column 1: syntax error while parsing value - unexpected end of input; expected '[', '{', or a literal
run.sh: line 9: 499867 Aborted                 (core dumped) ./main $*

which is 'logical' given that I should have typed bash run.sh network_L5bPyrCellHayEtAl2011 but still the error is confusing.

[llandsmeer]

Another bug: if you execute main.py from another directory than where main.py resides it will fail. Arbor-build-catalogue creates the local-catalogue.so file in the current directory, but then main.py will try to load it from the here directory where main.py resides - which might not be the same directory..

[llandsmeer]

Why these changes?

[thorstenhater]

Clippy ;)

[llandsmeer]

Bundle cfg is a good idea

[llandsmeer]

Another bug when trying to convert the GLC network:

$ RUST_BACKTRACE=1 nmlcc bundle NeuroML2/MaexDeSchutter1998.net.nml gcl
thread 'main' panicked at 'no entry found for key', src/bundle.rs:158:33
stack backtrace:
   0: rust_begin_unwind
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/panicking.rs:142:14
   2: core::panicking::panic_display
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/panicking.rs:72:5
   3: core::panicking::panic_str
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/panicking.rs:56:5
   4: core::option::expect_failed
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/option.rs:1880:5
   5: nml2::bundle::export
   6: nmlcc::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

which is the line

                let threshold = cell_to_threshold[pre_cell_id];

where

cell_to_threshold = {"Golgi_98": -20.0, "Granule_98": -20.0}
pre_cell_id = "poisson_input"

Adding a continue statement later leads to

    mrf = self.cell_to_morph[cid]
KeyError: 'poisson_input'

[thorstenhater]

Hi @llandsmeer

as you note C++ support is still pretty rough. It also won't work without running the Python code first.

Regarding the Poisson Input problem, this arise since nmlcc doesn't model Spike Source Cells, yet.
Opened

#37