Re: src/ directory re-org

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: src/ directory re-org

Rik-4
On 08/01/2012 09:42 AM, [hidden email] wrote:

> Message: 2
> Date: Wed, 1 Aug 2012 12:27:44 -0400
> From: "John W. Eaton" <[hidden email]>
> To: octave maintainers mailing list <[hidden email]>
> Subject: more source directory reorganization
> Message-ID: <[hidden email]>
> Content-Type: text/plain; charset=us-ascii
>
> I pushed a few more source reorganization changes to savannah.
>
> You will need to re-run ./autogen.sh in your source tree after
> updating.
>
> If you don't want to have old files left over in your build tree, you
> can remove the src directory before running configure.
>
> If you are building in the source tree, you obviously can't take that
> simple step and will instead have to do something like make
> maintainer-clean in the src directory before updating the sources.
> But that also raises the question of why are you building in the
> source tree instead of using a separate directory for the build?
>
> The changes are:
>
>   * To be more consistent with the naming of the new src/corefcn
>     directory, src/DLD-FUNCTIONS is now src/dldfcn.
>
>   * The following files, which only define DEFUN functions and do not
>     export any functions have been moved to the corefcn directory:
>
>       bitfcns.cc  mappers.cc  sparse.cc  strfns.cc  syscalls.cc
>
>   * Since there should be no header files in the corefcn directory
>     (the only external functions in those files should be defined with
>     DEFUN) the -Icorefcn and -I$(srcdir)/corefcn flags have been
>     removed from the compiler flags in src/Makefile.am.
>
> There are a few more files in the src directory that contain a lot of
> DEFUN functions, but they also contain some non-DEFUN functions that
> are exported.  I'm thinking that we should split the DEFUN functions
> out from these other external functions and move them to corefcn.
> Ideally, corefcn would just contain DEFUN wrappers and the code that
> provides the functionality would be available to other C++ code
> without needing to call a DEFUN function.  For example,
> src/corefcn/time.cc is a good example of how things should be done,
> and src/corefcn/qz.cc is an example of what NOT to do.  Instead of
> doing the computations in the DEFUN, there should be a separate class
> (in liboctave, if possible) that implements the computation for the qz
> function and qz.cc should just decode arguments and use the class.
>
> Comments?
John,

There are still a few more changes I would make.  Running 'ls *.h *.cc | wc
-l' in the src/ directory returns 167 which is still a just barely
comprehensible menagerie for us mere mortals.  (You have demi-god status
and are excepted from this).  I would continue to divide the code up along
logical lines.  For example, I think the lexer and parser code should sit
with the other parser-related code in the parse-tree directory.  I also
think the various options setting code, like LSODE-opts.cc, should reside
in corefcn/.

I think the reasoning to divide up core library code from DEFUN code is
good.  In addition, I would like to make one more division between
interpreter-related functions (load, save, dbstop, etc.) and math-related
functions (pinv, chol, etc.).  I find this grouping easier to understand
than one based on whether or not there is a header file and I think it will
help prevent corefcn from becoming too large and ungainly.  So, my proposed
structure would be:

parse-tree/
  lexer, parser, and all parse-tree pt* code
octave-value/
  All octave_value related code ov*
operators/
  All operator related code
template-inst/
  Not sure about this one
core-interp/ (Name open to change)
  C++ code in liboctinterp which is called directly rather than through a
DEFUN.
interpfcn/ (Name open to change)
  DEFUN C++ code for interpreter functions such as load, save, dbstop.
corefcn/
  DEFUN C++ code for mathematical core functions such as pinv, chol.
dldfcn/
  Dynamically loaded functions

src/
  Remaining code files, mostly for octave itself such as main.c, octave.cc.
  Would also potentially contain a few scripts like mkbuiltins.

--Rik
Reply | Threaded
Open this post in threaded view
|

Re: src/ directory re-org

John W. Eaton
Administrator
On  2-Aug-2012, Rik wrote:

| I think the reasoning to divide up core library code from DEFUN code is
| good.  In addition, I would like to make one more division between
| interpreter-related functions (load, save, dbstop, etc.) and math-related
| functions (pinv, chol, etc.).  I find this grouping easier to understand
| than one based on whether or not there is a header file and I think it will
| help prevent corefcn from becoming too large and ungainly.  So, my proposed
| structure would be:
|
| parse-tree/
|   lexer, parser, and all parse-tree pt* code
| octave-value/
|   All octave_value related code ov*
| operators/
|   All operator related code
| template-inst/
|   Not sure about this one
| core-interp/ (Name open to change)
|   C++ code in liboctinterp which is called directly rather than through a
| DEFUN.
| interpfcn/ (Name open to change)
|   DEFUN C++ code for interpreter functions such as load, save, dbstop.
| corefcn/
|   DEFUN C++ code for mathematical core functions such as pinv, chol.
| dldfcn/
|   Dynamically loaded functions
|
| src/
|   Remaining code files, mostly for octave itself such as main.c, octave.cc.
|   Would also potentially contain a few scripts like mkbuiltins.

I'm OK with this change.

Code for load and save is also included in the octave_value objects.
the ls-*.{h,cc} files are for file formats that are limited to
specific data types, like the Matlab MAT file format.

Would corefcn, interpfcn, and dldfcn contain any header files to
export functions that are called directly?

If we split files apart, what should we do about copyright info when
there are multiple people claiming copyright?

I think template-inst should be removed, if that is now possible.  It
is a holdover from when we needed to instantiate templates manually.
Now we mostly don't seem to need to do that, but I'm not sure what the
rules are for instantiation now.

In addition to the src/template-inst directory, we also have some
template instantiation files in liboctave.  For example:

  Array-b.cc     Array-C.cc     Array-ch.cc     Array-d.cc
  Array-f.cc     Array-fC.cc    Array-i.cc      Array-idx-vec.cc
  Array-s.cc     Array-str.cc   Array-voidp.cc  MArray-C.cc
  MArray-d.cc    MArray-f.cc    MArray-fC.cc    MArray-i.cc
  MArray-s.cc    MSparse-C.cc   MSparse-d.cc    Sparse-b.cc
  Sparse-C.cc    Sparse-d.cc


jwe
Reply | Threaded
Open this post in threaded view
|

Re: src/ directory re-org

Michael Goffioul
On Thu, Aug 2, 2012 at 8:40 PM, John W. Eaton <[hidden email]> wrote:
On  2-Aug-2012, Rik wrote:

| I think the reasoning to divide up core library code from DEFUN code is
| good.  In addition, I would like to make one more division between
| interpreter-related functions (load, save, dbstop, etc.) and math-related
| functions (pinv, chol, etc.).  I find this grouping easier to understand
| than one based on whether or not there is a header file and I think it will
| help prevent corefcn from becoming too large and ungainly.  So, my proposed
| structure would be:
|
| parse-tree/
|   lexer, parser, and all parse-tree pt* code
| octave-value/
|   All octave_value related code ov*
| operators/
|   All operator related code
| template-inst/
|   Not sure about this one
| core-interp/ (Name open to change)
|   C++ code in liboctinterp which is called directly rather than through a
| DEFUN.
| interpfcn/ (Name open to change)
|   DEFUN C++ code for interpreter functions such as load, save, dbstop.
| corefcn/
|   DEFUN C++ code for mathematical core functions such as pinv, chol.
| dldfcn/
|   Dynamically loaded functions
|
| src/
|   Remaining code files, mostly for octave itself such as main.c, octave.cc.
|   Would also potentially contain a few scripts like mkbuiltins.

I'm OK with this change.

Code for load and save is also included in the octave_value objects.
the ls-*.{h,cc} files are for file formats that are limited to
specific data types, like the Matlab MAT file format.

Would corefcn, interpfcn, and dldfcn contain any header files to
export functions that are called directly?

If we split files apart, what should we do about copyright info when
there are multiple people claiming copyright?

I think template-inst should be removed, if that is now possible.  It
is a holdover from when we needed to instantiate templates manually.
Now we mostly don't seem to need to do that, but I'm not sure what the
rules are for instantiation now.

In addition to the src/template-inst directory, we also have some
template instantiation files in liboctave.  For example:

  Array-b.cc     Array-C.cc     Array-ch.cc     Array-d.cc
  Array-f.cc     Array-fC.cc    Array-i.cc      Array-idx-vec.cc
  Array-s.cc     Array-str.cc   Array-voidp.cc  MArray-C.cc
  MArray-d.cc    MArray-f.cc    MArray-fC.cc    MArray-i.cc
  MArray-s.cc    MSparse-C.cc   MSparse-d.cc    Sparse-b.cc
  Sparse-C.cc    Sparse-d.cc

AFAIK the reason for having manual instantiation is because part of the template code is not in the header files. I'm not sure what's the reason to have part of the template codes in .cc, maybe to make compilation lighter?

Michael.

Reply | Threaded
Open this post in threaded view
|

Re: src/ directory re-org

John W. Eaton
Administrator
On  2-Aug-2012, Michael Goffioul wrote:

| AFAIK the reason for having manual instantiation is because part of the
| template code is not in the header files.

OK, that makes sense.

| I'm not sure what's the reason to
| have part of the template codes in .cc, maybe to make compilation lighter?

Assuming that we avoid doing some work to generate code for those
functions that are only declared and not defined, then we probably do
save some time.  Array.cc is fairly complicated and 2800 lines long.
Array.h is already 750 lines.  I guess the only way to really find out
is to try combining Array.h and Array.cc and see what happens.

jwe
Reply | Threaded
Open this post in threaded view
|

Re: src/ directory re-org

Rik-4
In reply to this post by John W. Eaton
On 08/02/2012 12:40 PM, John W. Eaton wrote:

>
> Would corefcn, interpfcn, and dldfcn contain any header files to
> export functions that are called directly?
I think we will need to wait and see how things evolve.  The dldfcn/
directory contains just one header file, oct-qhull.h, but that is for its
own use and not for exporting functions.  This leads me to believe there it
at least a possibility that these three directories would not need to
export functions.
>
> If we split files apart, what should we do about copyright info when
> there are multiple people claiming copyright?
I think the safest thing is to assign copyright from all authors to both
split files.  If there was a practical way of tracing authorship then we
could extend copyright only on the portions that a given author had
written.  In the print world the analogy might be to an anthology of short
stories.  In that case, each chapter bears its own copyright and if one
were to take multiple chapters it would be fairly straightforward to just
list the authors on the now reduced collection of stories.  In this case,
since I doubt we can disentangle which stanzas of code were written by
which particular author, I think we must list all authors as having
contributed to the new source file.

I also think this sort of change would be better as a second phase.  The
first phase I see is just to deal the whole files out between
subdirectories.  And after that we can slowly decide whether a file belongs
in a different directory or whether it should be split into multiple files.

--Rik


Reply | Threaded
Open this post in threaded view
|

Re: src/ directory re-org

John W. Eaton
Administrator
On  2-Aug-2012, Rik wrote:

| On 08/02/2012 12:40 PM, John W. Eaton wrote:
|
| >
| > Would corefcn, interpfcn, and dldfcn contain any header files to
| > export functions that are called directly?
| I think we will need to wait and see how things evolve.  The dldfcn/
| directory contains just one header file, oct-qhull.h, but that is for its
| own use and not for exporting functions.  This leads me to believe there it
| at least a possibility that these three directories would not need to
| export functions.

OK.

| > If we split files apart, what should we do about copyright info when
| > there are multiple people claiming copyright?
| I think the safest thing is to assign copyright from all authors to both
| split files.  If there was a practical way of tracing authorship then we
| could extend copyright only on the portions that a given author had
| written.  In the print world the analogy might be to an anthology of short
| stories.  In that case, each chapter bears its own copyright and if one
| were to take multiple chapters it would be fairly straightforward to just
| list the authors on the now reduced collection of stories.  In this case,
| since I doubt we can disentangle which stanzas of code were written by
| which particular author, I think we must list all authors as having
| contributed to the new source file.

Yes, I agree that this is probably the best we can do.

| I also think this sort of change would be better as a second phase.  The
| first phase I see is just to deal the whole files out between
| subdirectories.  And after that we can slowly decide whether a file belongs
| in a different directory or whether it should be split into multiple files.

I'm OK with going ahead with the reorganization even if that means
that we need to add -IDIR -I$(DIR) to the list of CPP flags in the
src/Makefile for each of these new subdirectories, with the goal being
to eventually make some of the directories contain only DEFUN
functions.  I think the added structure is worth more than the clutter
of extra flags in the compiler commands.

jwe