arbor-sim / gui

Other
2 stars 6 forks source link

Bug/lukas acc #14

Closed thorstenhater closed 2 years ago

thorstenhater commented 2 years ago

Show an error on ACC loader error instead of crashing.

lukasgd commented 2 years ago

Thanks for the clarifications on the explicit qualification of the catalogue (fix by hh --> default::hh). The lukasgd-segv-apr22.acc you committed still causes the segfault, though. Was that intentional? And how do you suggest to output GUI-compatible ACC from arbor itself?

thorstenhater commented 2 years ago

![Uploading Screenshot 2022-04-12 at 12.12.45.png…]()

If this upload finishes, this is what I see: An error is shown telling you it cannot load this ACC.

lukasgd commented 2 years ago

Ok, will need to debug it further as I'm still getting that segfault (despite building the GUI on c25f8ef).

lukasgd commented 2 years ago

I'm already getting a segfault at https://github.com/arbor-sim/gui/blob/452c3250585035f0ac90b033cd814b05d86995a9/src/gui_state.cpp#L1113

This doesn't throw an exception if the range to erase is past the end (which it is). A possible fix that works for me would be adding

      { auto sep = std::find(name.begin(), name.end(), ':'); name.erase(name.begin(), sep == name.end() ? name.begin() : sep + 2); }
      { auto sep = std::find(cat.begin(),  cat.end(),  ':'); cat.erase(sep == cat.end() ? cat.begin() : sep, cat.end()); }

The second line is not strictly necessary, just for consistency - the example will then fail with

[2022-04-12 17:28:44.675] [debug] Failed to load ACC: Unknown catalogue:

Probably the help message is more useful if the second line (cat) remains unchanged.

thorstenhater commented 2 years ago

I added a more sane splitting/parsing scheme.

lukasgd commented 2 years ago

That's great, thanks! Is there any idea on how to resolve the incompatibility of GUI-compatible ACC from that output by Arbor itself?

thorstenhater commented 2 years ago

Sure! Just use the prefix argument to import:

auto cat = arb::mechanism_catalogue{}; // Empty
catalogue.import(arb::default_catalogue{}, "default::"); // Add default w/ prefix
// etc

and from there use paint and export accordingly. Does this work?