Skip to content

Commit

Permalink
Add better (helpful?) errors on mechanism semantic misuse. (#2171)
Browse files Browse the repository at this point in the history
Tell users if they switch up `GLOBAL` and 'normal' `PARAMETER`s while
placing/painting.
Example
```
In [3]: d = A.decor(); d.paint('(all)', A.density('pas/g=17')); c = A.cable_cell(A.segment_tree(), d); s = A.single_cell_mod
   ...: el(c); s.run(1)
---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)
Cell In[3], line 1
----> 1 d = A.decor(); d.paint('(all)', A.density('pas/g=17')); c = A.cable_cell(A.segment_tree(), d); s = A.single_cell_model(c); s.run(1)

RuntimeError: mechanism 'pas' has no global parameter 'g', but a normal parameter with the same name exists. Set it via the parameter map, eg 'density("pas", {{"g", ...}, ...})'

In [7]: d = A.decor(); d.paint('(all)', A.density('pas', {'e':17})); t = A.segment_tree(); t.append(A.mnpos, (0, 0, 0, 1), (
   ...: 0, 0, 1, 1), 1); c = A.cable_cell(t, d); s = A.single_cell_model(c); s.run(1)
---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)
Cell In[7], line 1
----> 1 d = A.decor(); d.paint('(all)', A.density('pas', {'e':17})); t = A.segment_tree(); t.append(A.mnpos, (0, 0, 0, 1), (0, 0, 1, 1), 1); c = A.cable_cell(t, d); s = A.single_cell_model(c); s.run(1)

RuntimeError: mechanism 'pas' has no parameter 'e', but a global parameter with the same name exists. Use 'pas/e=...' to set it.
```
  • Loading branch information
thorstenhater authored Aug 7, 2023
1 parent 7d36beb commit 0a38196
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 6 deletions.
20 changes: 20 additions & 0 deletions arbor/arbexcept.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,26 @@ no_such_parameter::no_such_parameter(const std::string& mech_name, const std::st
param_name(param_name)
{}

did_you_mean_global_parameter::did_you_mean_global_parameter(const std::string& mech_name, const std::string& param_name):
arbor_exception(pprintf("mechanism '{}' has no parameter '{}', "
"but a global parameter with the same name exists. "
"Use '{}/{}=...' to set it.",
mech_name, param_name,
mech_name, param_name)),
mech_name(mech_name),
param_name(param_name)
{}

did_you_mean_normal_parameter::did_you_mean_normal_parameter(const std::string& mech_name, const std::string& param_name):
arbor_exception(pprintf("mechanism '{}' has no global parameter '{}', "
"but a normal parameter with the same name exists. "
"Set it via the parameter map, eg 'density(\"{}\", {{\"{}\", ...}, ...})'",
mech_name, param_name,
mech_name, param_name)),
mech_name(mech_name),
param_name(param_name)
{}

illegal_diffusive_mechanism::illegal_diffusive_mechanism(const std::string& m, const std::string& i):
arbor_exception(pprintf("mechanism '{}' accesses diffusive value of ion '{}', but diffusivity is disabled for it.", m, i)),
mech{m},
Expand Down
9 changes: 8 additions & 1 deletion arbor/fvm_layout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,14 @@ void verify_mechanism(const ion_species_map& global_ions,
const mechanism_desc& desc) {
const auto& name = desc.name();
for (const auto& [p, v]: desc.values()) {
if (!info.parameters.count(p)) throw no_such_parameter(name, p);
if (!info.parameters.count(p)) {
if (info.globals.count(p)) {
throw did_you_mean_global_parameter(name, p);
}
else {
throw no_such_parameter(name, p);
}
}
if (!info.parameters.at(p).valid(v)) throw invalid_parameter_value(name, p, v);
}

Expand Down
12 changes: 12 additions & 0 deletions arbor/include/arbor/arbexcept.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,18 @@ struct ARB_SYMBOL_VISIBLE no_such_parameter: arbor_exception {
std::string param_name;
};

struct ARB_SYMBOL_VISIBLE did_you_mean_global_parameter: arbor_exception {
did_you_mean_global_parameter(const std::string& mech_name, const std::string& param_name);
std::string mech_name;
std::string param_name;
};

struct ARB_SYMBOL_VISIBLE did_you_mean_normal_parameter: arbor_exception {
did_you_mean_normal_parameter(const std::string& mech_name, const std::string& param_name);
std::string mech_name;
std::string param_name;
};

struct ARB_SYMBOL_VISIBLE illegal_diffusive_mechanism: arbor_exception {
explicit illegal_diffusive_mechanism(const std::string& mech, const std::string& ion);
std::string mech;
Expand Down
15 changes: 10 additions & 5 deletions arbor/mechcat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,17 +292,22 @@ struct catalogue_state {

// Update global parameter values in info for derived mechanism.

for (const auto& kv: global_params) {
const auto& param = kv.first;
const auto& value = kv.second;

for (const auto& [param, value]: global_params) {
if (auto* p = ptr_by_key(new_info->globals, param)) {
if (!p->valid(value)) {
return unexpected_exception_ptr(invalid_parameter_value(name, param, value));
}
}
else {
return unexpected_exception_ptr(no_such_parameter(name, param));
if (new_info->parameters.count(param)) {
return unexpected_exception_ptr(
did_you_mean_normal_parameter(name.substr(0,
name.find('/')),
param));
}
else {
return unexpected_exception_ptr(no_such_parameter(name, param));
}
}

deriv.globals[param] = value;
Expand Down
32 changes: 32 additions & 0 deletions test/unit/test_fvm_layout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1561,6 +1561,38 @@ TEST(fvm_layout, ion_weights) {
}
}

TEST(fvm_layout, global_vs_range_parameters) {
soma_cell_builder builder(5);
builder.add_branch(0, 100, 0.5, 0.5, 1, "dend");

{
auto desc = builder.make_cell();
desc.decorations.paint("dend"_lab, density("pas", {{"e", 42}}));

EXPECT_THROW(
fvm_build_mechanism_data({},
{{desc}},
{0},
{{}},
fvm_cv_discretize({{desc}},
neuron_parameter_defaults)),
did_you_mean_global_parameter);
}
{
auto desc = builder.make_cell();
desc.decorations.paint("dend"_lab, density("pas/g=23"));

EXPECT_THROW(
fvm_build_mechanism_data({},
{{desc}},
{0},
{{}},
fvm_cv_discretize({{desc}},
neuron_parameter_defaults)),
did_you_mean_normal_parameter);
}
}

TEST(fvm_layout, revpot) {
// Create two cells with three ions 'a', 'b' and 'c'.
// Configure a reversal potential mechanism that writes to 'a' and
Expand Down

0 comments on commit 0a38196

Please sign in to comment.