[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Code integration into Octave core of ilu and ichol functions.

From: Kai Torben Ohlhus
Subject: Re: Code integration into Octave core of ilu and ichol functions.
Date: Wed, 13 Aug 2014 17:34:19 +0200

On Wed, Aug 13, 2014 at 12:29 AM, Eduardo <address@hidden> wrote:
2014-08-12 23:09 GMT+01:00 Kai Torben Ohlhus <address@hidden>:

On Tue, Aug 12, 2014 at 6:45 PM, Eduardo <address@hidden> wrote:
Hi Kai,

I have moved the code from my personal repository to a new Octave subrepo I have set on bitbucket. I made the changes necessary for adapting the code to compile within Octave development branch. The locations of the files in the source code tree  are:

(1) libinterp/dldfcn for ilu0.cc iluc.cc ilutp.cc ichol0.cc and icholt.cc files (dld functions)
(2) scripts/sparse for ilu.m and iluc.m

I chose those places because I saw (looking at your repository) you did last year this way. The new subrepo can be accessed from here [1]. 

I also have some questions about the way you managed the visibility of dld functions in terms of being able or not to call them from the interpreter. I noticed that your approach last year was to let the functions in location (1) be callable from the interpreter, maybe because they offered some extra functionality that was not used by the wrapper functions (ilu.m and ichol.m). In my case they do not.  So maybe a good idea would be to make them in some way private so they can be accessed by the wrappers ilu.m and ichol.m but not callable from the interpreter. Is this the way to go? If so, how can this be done?

[1] https://address@hidden/edu159/octave-edu159



Hi Eduardo,

Regarding your question, you're right. I chose to let the individual functions to be callable to someone who has a deeper knowledge about the factorizations and wants to directly apply them. However, when I take a look in the folder dldfcn, the current convection seems to be creating a __filename__.cc file for internal functions that are not intended to be used. But they are not hidden then that is for sure. I don't know if a "private" subdirectory makes sense in this case, like it is done for m-Files.


I have taken a look to __foo__.cc kind of functions and you are right, they are not hidden and can be called from the interpreter. That is a bit ugly IMO. Anyway, any of the maintainers knows which would be the best way to go?  

Thanks in advance.


Hi Eduardo,

I had a closer look to your code and here is my first good impression:

octave:15> test ilu
PASSES 6 out of 6 tests
octave:16> test ilu0
PASSES 21 out of 21 tests
octave:17> test iluc
PASSES 24 out of 24 tests
octave:18> test ilutp
PASSES 31 out of 31 tests
octave:19> test ichol
PASSES 21 out of 21 tests
octave:20> test ichol0
PASSES 17 out of 17 tests
octave:21> test icholt
PASSES 26 out of 26 tests

Congratulations, I think your code is ready to get into the main development repository!

But nevertheless, there are some things to improve:

1) In ilutp.cc there are some test cases for iluc
2) Check the formatting in all files (maybe use tools like http://astyle.sourceforge.net/ or https://www.gnu.org/software/indent/ for this work)
3) There are a few typos in the comments

Regarding the big issue with the hiding of sub-functions like ilu0.cc, simply take for now the __filename__.cc pattern, until we hear of a better idea. Then you can proceed to your final step, the Octave Changeset. Therefore read these carefully (again):


Finally submit your work as a Changeset to the patch tracker.


Until the end of GSoC you can still try to improve the documentation and the test cases, what is the intention of the soft pencils down. But the majority of you work is done I think.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]