Consistency in Naming

[thorstenhater]

Currently we have a few hooplas in C++/Python API consistency:

Current clamps

i_clamp ./. iclamp: Either the C++ or the Python member needs to be renamed.

In Python iclamp(1, 2, 3) will give a 3nA current starting at 1ms and ending at 3ms. In C++ i_clamp(1, 2, 3) will result in a 1nA current with a sine profile of 2kHz phase shifted by 3rad. Python's behaviour is handled by i_clamp::box in C++. Clearly, this needs to change.

Morphology

Some morphology primitive have the m prefix, some not.

Things to consider

• remove the m prefix, ie mcable -> cable
• add it to Python consistently
• introduce a Python submodule for morphology types
• make all primitives interchangeable with tuples, ie cable { branch, from, to} <> (branch, from, to)

[thorstenhater]

@Helveg maybe something for you?

[Helveg]

make all primitives interchangeable with tuples, ie cable { branch, from, to} <> (branch, from, to)

I kind of like that arbor is slightly terse there. It's very consistent and explicit. If you allow this interchangability I think it will mostly just increase the cognitive load. What advantage do you see from it?

@Helveg maybe something for you?

I can try, but if my IDE's refactor button doesn't find an occurence, neither will I ;)

[thorstenhater]

We do allow this conversion for mpoint, so, consistency. But I agree, explicit beats implicit.

rg -t cpp 'class_<m' python/ should turn up all the candidates ;)

[thorstenhater]

Added an even more annoying example.

[halfflat]

My vague voice from beyond says: I'd like to keep the 'm' prefix because the terms segment etc. are so overloaded in the literature that leaving the 'm' off invites more confusion. But with iclamp in particular, we should offer the C++ functionality in the Python API, ideally with the same names, and definitely with the same parameter orders.

[thorstenhater]

Hey @halfflat,

thanks for the input. I agree on the conceptual overload and the need for disambiguation. These items could alternatively live in arb::morph, making opting out of the prefix a possibility (and in Python arbor.morph. Main point here though is consistency. From your and Robin's comment I think the consensus would be to use the m prefix consistently in both Python and C++.

About i_clamp: I'd personally prefer to adjust the C++ API to the Python API, reasoning here is that we have a lot more models in Python than in C++. And some tests were actually using the Python style c'tors in the incorrect belief (comment!) they were identical. Ideal solution here would be this

// NB. consistent naming with Python
iclamp(mA amplitude, Hz freq=0, rad=phase);
iclamp(ms from, ms duration, mA amplitude, Hz freq=0, rad phase=0);
iclamp([(ms, mA)] envelope, Hz freq=0, rad phase=0);

Problem being, the first and second c'tors are ambiguous, so this would be my compromise:

// NB. consistent naming with Python
iclamp(mA amplitude);
iclamp(ms from, ms duration, mA amplitude, Hz freq=0, rad phase=0);
iclamp([(ms, mA)] envelope, Hz freq=0, rad phase=0);

although still a bit miffy. If I were designing this from scratch, I'd put the amplitude first 🤷. Python is a bit easier

iclamp(ms from=0, ms duration=forever, mA amplitude=0, **, Hz freq=0, rad phase=0)
iclamp([(ms, mA)] envelope, **, Hz freq=0, rad phase=0)

Again, I'd love to put the amplitude first, but that'd break too much existing code in my book.

[halfflat]

If we're aligning the APIs, we're going to have a version incompatibility regardless; perhaps it is better to break both in a way that will fail to compile (C++) or raise an exception if the old API is used?

We could throw out the i_clamp::i_clamp overloads and use factories such as i_clamp::box (Python equivalent i_clamp_box?), i_clamp::constant (or maybe steady) and i_clamp::envelope. Then we could consistently have amplitude first, for example. Instead of factories, could add a tag type to i_clamp ctor or overload the ctor with different parameter structs, keeping things typey and reducing the risk of parameter order mixups (e.g. i_clamp::i_clamp(const i_clamp_envelope&), i_clamp::i_clamp(const i_clamp_box&) etc.).

As things stand, I think the C++ API is currently the saner of the two though.