mcdougallab / matlabneuroninterface

Interface for connecting NEURON and MATLAB
BSD 3-Clause "New" or "Revised" License
5 stars 1 forks source link

Command-line interface (CLI) usability enhancements #84

Closed vijayiyer05 closed 8 months ago

vijayiyer05 commented 1 year ago

Umbrella issue for exploring/tracking/dispensing ideas to enhance the consistency & usability of the NEURON Toolbox command-line interface (CLI)

vijayiyer05 commented 1 year ago
ramcdougal commented 1 year ago

WRT neuron.Session, my only hesitation is that it might imply you could have more than one NEURON session, which you can't do at this point.

vijayiyer05 commented 1 year ago

WRT neuron.Session, my only hesitation is that it might imply you could have more than one NEURON session, which you can't do at this point.

Given this limitation, I would suggest to use the Singleton pattern for the class. In short: make constructor private & make a static method with a persistent variable.

edovanveen commented 11 months ago
vijayiyer05 commented 11 months ago

Re naming conventions, a few patterns can be seen from averaging across various classes implemented in MathWorks products:

vijayiyer05 commented 11 months ago

Worth taking a look at the 'new' subsref: https://www.mathworks.com/help/matlab/ref/matlab.mixin.indexing.redefinesdot-class.html

Suggest to try it for 1-2 of the classes, if it's straightforward

hvangeffen commented 11 months ago

Could also take a look at https://nl.mathworks.com/help/matlab/ref/libpointer.html and related pages of matlab working with c to see how they define their interfaces

vijayiyer05 commented 10 months ago
  • [ ] Rename neuron.Neuron to neuron.Session & add separate function to start a session

For the separate function, how about keeping it simple such as neuron.launch. Given it's a singleton, I don't think there's a need to put 'Session' in the function name.

vijayiyer05 commented 9 months ago
  • [ ] Rename neuron.Neuron to neuron.Session & add separate function to start a session

For the separate function, how about keeping it simple such as neuron.launch. Given it's a singleton, I don't think there's a need to put 'Session' in the function name.

Looking back at this, I should clarify: main thought was that I don't think it's a golden rule to use 'session' in the Session object factory method name even though that's the name of the object returned. 'Launch' came to mind first but that's not the main point.

ramcdougal commented 9 months ago

I'm still confused by the rationale for this. It's going to be a someone for the foreseeable future. What's the advantage of returning a Singleton that requires a separate initialization step as opposed to just returning it initialized? Splitting it in two just seems like extra work.

vijayiyer05 commented 9 months ago

I'm still confused by the rationale for this. It's going to be a someone for the foreseeable future. What's the advantage of returning a Singleton that requires a separate initialization step as opposed to just returning it initialized? Splitting it in two just seems like extra work.

I agree the constructor can 'know' that it's a singleton & just initialize itself if sensible (as it is here). No need for a separate initializer function. But that doesn't address the multiple constructor call matter. Note the constructor is typically hidden from the user in the singleton pattern, i.e., there's only one function to know.

Personally I wish there were a way to implement a Singleton constraint without a separate factory method. But currently a MATLAB constructor has no way to enforce that it's the only one.

That said, can you remind me if NEURON itself implements a singleton constraint? If only one NEURON session is possible under the hood, it's likely possible to avoid re-implementing a singleton constraint in the MATLAB wrapper.

edovanveen commented 8 months ago

Status update:

edovanveen commented 8 months ago

@ramcdougal ready for review/merge

vijayiyer05 commented 8 months ago

@edovanveen, great that you've made progress on the command chaining. Is there a particular script file you can point me to where some syntax was improved or made possible by this improved command chaining?

edovanveen commented 8 months ago

@vijayiyer05 Yes, you can take a look at subsref in one of the c++ object wrapper classes (for example Object.m). After taking care of the input (the first element, or the first two elements of S), the function neuron.chained_method() is called, which does: [varargout{1:nargout}] = varargs{:}.subsref(S(n_processed+1:end));. In other words, subsref is called iteratively, until its input argument S runs out of elements to process.

edovanveen commented 8 months ago

@vijayiyer05 Ah it seems that I misunderstood your question. An example of syntax that is possible now is:

>>n = neuron.launch();
>>v_max = n.Vector().append(10).append(11).append(12).max();
>>disp(v_max);
     12

This example is from example_vector.m.

Previously, this would have had to be something like:

>>n = neuron.launch();
>>v = n.Vector();
>>v.append(10);
>>v.append(11);
>>v.append(12);
>>v_max = v.max();
>>disp(v_max);
     12
vijayiyer05 commented 8 months ago

Thank you Edo for explaining. And I can see the added chained call in example.m.

I could imagine some subjective disagreement on whether such a degree of chaining is advisable. In your experience (or that of @ramcdougal) would such chaining be common syntax for a Python user?

If the main advantage is making things a one-liner, it could be done as: >> v=n.Vector(); v.append(10); v.append(11); v.append(12); v_max = v.max()

Are there other more complex (& realistic) use cases you (or @ramcdougal) can imagine where compound indexing is more 'necessary'?

As an example, one good use case for subsref (or more recently: customized object indexing) in my experience is navigating a complex/nested data structure with a mix of indexes and field names to reach the desired subtree or leaf. Without compound indexing, one has to create various intermediate variables & the semicolon-delimited one-liner approach above becomes untenable.

ramcdougal commented 8 months ago

It's of course never literally necessary. However, sometimes it is convenient to allow things that one might conceptualize as single concepts to be expressed in a single statement.

(I would argue that multiple statements on one line don't convey the same conceptual relatedness that one single statement conveys.)

The classic example that people use all the time in Python is to create a Vector and tell it to record some thing:

v = h.Vector().record(soma(0.5)._ref_t)

When you invoke a method on an object you don't necessarily return a modified version of an object; you may return a new object.

Being used to other programming languages, I don't understand why it would never not be allowed. Whether or not it should be used in a specific context is a different question.

vijayiyer05 commented 8 months ago

Thanks @ramcdougal for your thoughts on this. Sorry I didn't catch it sooner.

Interesting re the importance you see of chaining for conceptual relatedness.

When you say methods can either return a modified version or a new object, can you comment on what's more typical in Python in your experience? On the MATLAB side, yes both could be true, but it's always one or the other for a given class, i.e., a class is either a handle class or a value class.

I haven't fully internalized the general case of dot notation to answer your broader puzzle, but what comes to mind is that a method (or property access) by dot notation could return a non-object structure or an object of a different type. So overall 'recursive chaining' seems only feasible in some circumstances.

Still interested to hear thoughts from @edovanveen on this. Again, thanks for weighing in!

edovanveen commented 8 months ago

@vijayiyer05 All supported Neuron functions (for now, at least) can only return a string, a double, or another Neuron object. So as long as all matlab wrappers support chained method calls, there should not be an issue.

ramcdougal commented 8 months ago

The return type shouldn't matter unless this is a subsref limitation. That which comes after the dot acts on whatever object the previous item returned.

In Python, the PlotShape.plot method returns an instance of a plotly or matplotlib graph, and so anything that's dotted on to that would have to be an e.g. plotly graph method not a PlotShape method.

edovanveen commented 8 months ago

@ramcdougal in this case, subsref-chaining does not work on classes without a subsref implementation... Because dynamic methods don't actually exist in matlab.

vijayiyer05 commented 8 months ago

Great discussion, thank you both!