Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parser refactoring #22

Open
pausz opened this issue Aug 14, 2015 · 1 comment
Open

Parser refactoring #22

pausz opened this issue Aug 14, 2015 · 1 comment

Comments

@pausz
Copy link
Contributor

pausz commented Aug 14, 2015

From @RomeshA

Related to #19

If we're changing the parser, I wonder if it would be better to
separate the dendrites from the populations - there are as many of
them as there are couples, of course. They are presently contained
within the population blocks because of the way that the config file
is currently read, but I think this makes the config file more
confusing to read.

@pausz pausz added this to the Mark II milestone Aug 14, 2015
@felixfung
Copy link
Collaborator

Hi Romesh,

As has happened on occasions, we disagree on some design issues.

The interface should conform to the code, the code does not conform to the
interface.

It should be easy to write matlab or python interface (even gui). These
would be interfaces to an interface, the config file. Remember one of the
strengths of NeuroField is the config file and the option for direct use,
similar to outputting.

Manual config file is an important option, with many collaborators sharing
and reading it directly. Therefore, the integrity and presentation of it
must be kept. Imagine looking directly at an eirs config file, without
arbitrary white space to align columns, config files are very unreadable,
and new line delimiter definitely makes it unprintable.

C++ code should be separate and independent from other scripts. It is
unreasonable to refactor big chunks of code to accommodate an interface to
an interface. In this particular case, I don't even see how strict/loose
parameter ordering has got to do with matlab parsing. If you want a
"general" matlab or python interface, the best option is to require the
developer to supply the parameter option in a specific format, which the
matlab/python interface reads off. This way, code integrity/flexibility
will not be compromised, while strongly encouraging up-to-date
documentation.

Concerning what a logical block in a config file is: this is exactly the
problem. With the object hierarchy and inheritance structure, there are
always sub-objects within objects, and derived classes calling base
classes. The reader/writer of the config file should not worry about what
are the objects. The coder certain CANNOT foresee or rule out re-using
existing code, or the current code in development being re-used in the
future. The current "stupid" way does the job quite well. I challenge you
to find another way without exploding future development time and API
friendliness, as well as minimizing a user's chance of stupid mistakes
(which may be quite subtle and take a long time to find out!).

Labeling of populations and connections may be done, but there is no need
to change the underlying vector (Array) structure. An auxiliary (vector
based) object in Solver that reads letters from the connection matrix:

Connection matrix:
To: e i r s n
From e: 0 0 0 0 0
From i: 0 0 0 0 0
...

so that populations 1,2,3,4,5 has IDs e i r s n. Correctly calling the init
and step functions for each object is simple.

Regarding dendrites, perhaps some others would disagree with you concerning
confusion. In TMS modeling the current presentation actually helped some
collaborators in clarifying certain modeling issues. From a coding
standpoint, Dendrites is an object in QResponse which is an object in
Population. To have the interface otherwise would involve breaking
encapsulation and creating auxiliary objects.

If we're changing the parser, I wonder if it would be better to

separate the dendrites from the populations - there are as many of
them as there are couples, of course. They are presently contained
within the population blocks because of the way that the config file
is currently read, but I think this makes the config file more
confusing to read.

There should be a way to name populations and give them a single
letter subscript, which would then enable us to refer to populations
with labels like 'e','r','s', and to connections like 'ee','ei'...
rather than population 1, couple 3. This would also make it easier to
compare similar components when the model is changing and thus the
indexes might change.

I could imagine a solution using something like Python dictionaries to have

key:value pairs, where the key would be 'ee', 'ei' for connections and 'e',
'i'. I think numeric indexing is more general, so labels are mainly a human
readable decoration. Maybe we can generate a mapping between the two?
Apparently the equivalent dictionary structure in C++ is called std::map
http://en.cppreference.com/w/cpp/container/map. However, I've never
used it.

The order in which parameters are entered matters, and the order is

determined by the ordering of the statements in the init() function in
the cpp files, which means that there is no way to know what the
correct order is without inspecting the file.
Problems:
This makes it impossible
to implement the general parser without somehow modifying it to store
the correct order. The solution to this must be to make the config
files order-invariant. One possibility would be to rewind the file
pointer after each parameter is read, back to the original position
from which the init function was called. We would need to check
carefully what conditions this might impose on having duplicate or
similar parameter names. In this case, we would still need to somehow
return from the init function with the file pointer at the end of the
block (in preparation for the next block) - this could be difficult.
Potential Solution:
One option is to make the config file whitespace-dependent (and then
use newlines to separate logical blocks)?
Questions:
What's a logical block in the config file? An object with its parameters?

A general Matlab interface needs to be able to specify options for

any NeuroField object. However, the parameters of the object are
different for each of the classes, as well as for derived classes.
Therefore, the Matlab interface needs to be able to generate strings
for arbitrary parameters, which should be fine because the parameters
follow a standard syntax (with colons and dashes).

On Fri, Aug 14, 2015 at 12:24 AM, Paula notifications@github.com wrote:

From @RomeshA https://github.com/RomeshA

Related to #19 #19

If we're changing the parser, I wonder if it would be better to
separate the dendrites from the populations - there are as many of
them as there are couples, of course. They are presently contained
within the population blocks because of the way that the config file
is currently read, but I think this makes the config file more
confusing to read.


Reply to this email directly or view it on GitHub
#22.

Felix Fung, PhD
Postdoctoral Associate
-------------------------------------------
Department of Ophthalmology
Downstate Medical Center
State University of New York
450 Clarkson Avenue, MSC 58
Brooklyn, NY 11203, USA
---------------------------------------------
Office: +1 (718) 270-2100
Mobile: +1 (646) 683-8709
_E-mail: _fylixeoi@gmail.com fylixeoi@gmail.com
_E-mail: _ffung@physics.usyd.edu.au ffung@physics.usyd.edu.au

@pausz pausz removed this from the Mark II milestone Mar 9, 2017
@stuart-knock stuart-knock added this to the Backlog milestone Feb 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants