Suggestions
Norman
I have been looking at a Windows implementation of FoX with help from
Shane and I have some suggestions about FoX in general: please shoot
down in flames as necessary.
1) The code has lots of allocate and deallocate statements and few if
any are defended by 'stat=' and 'errmsg =' clauses, but the code does
have an error reporting system that these can be fed into.
2) The code could use a global 'verbose' logical scalar or vector,
which the user can set, so that progress and error reporting can be
made more or less accessible or invisible.
3) Can we dispense with the compiler directives and use a small config
file instead (written in XML of course!)?
4) Would there be any objections to refactoring the internal, private
code so that all module functions and subroutines have names that
start with the name of the module, I find this makes the code much
more readable. Calls to overloaded routines can then be spotted
easily by not conforming.
5) Can we review the extent to which recursive routines have been used
because, under IVF and Windows atleast, one can run out of stack space
very quickly.
Best wishes
N
Andrew Walker
Thanks for these. I've commented on each suggestion below. The major constraint on any of this the very small amount of time (of the order 1hr per month) I have to dedicate to FoX. This is OK as long as I'm just fixing the occasional bug but not enough to embark on anything more dramatic. For example, I've not made a release since February 2010 and defiantly need to do this as soon as I can. Patches are always welcome!
> On 7 Nov 2011, at 11:58, Norman wrote:
>
> Dear All,
>
> I have been looking at a Windows implementation of FoX with help from Shane and I have some suggestions about FoX in general: please shoot down in flames as necessary.
It would be very helpful if you could write up a short description of how you are compiling FoX on Windows and post it to this list. This is probably the most frequently asked question and all I can ever really say is that I think it's possible but I don't know how to do it.
> 1) The code has lots of allocate and deallocate statements and few if any are defended by 'stat=' and 'errmsg =' clauses, but the code does have an error reporting system that these can be fed into.
This is a good point. The way to approach this is probably to introduce a 'fox_allocate' subroutine that hides the error checking from the caller (to avoid the code becoming too much more convoluted) but checks that the underlying allocation was OK and makes use of the error reporting mechanism if it isn't. I wonder if this change could be made with the help of a script?
Another thing to flag up is that many of the allocations are within FoX's variable length string handling procedures. I have a rough version of FoX from 2009 that reworks this and reduces the number of allocations (speeding things up by a factor of ~10, if memory serves). However, I did introduce some bugs I've been unable to fix which is why this never got merged. This is probably something to sort out first, before working on the way that allocation is done.
> 2) The code could use a global 'verbose' logical scalar or vector, which the user can set, so that progress and error reporting can be made more or less accessible or invisible.
It would certainly be possible to extend FoX_set_fatal_warnings and friends in FoX_common to silence any warnings. One thing I had wondered about is allowing users to provide a callback function used to handle errors and warnings (for example, to plug into an applications own error handling subsystem).
I'm not sure exactly what progress information you want to see. If you are using SAX you could dump information out of, for example, a characters handling callback.
> 3) Can we dispense with the compiler directives and use a small config file instead (written in XML of course!)?
I don't really see how this could work. Are the compiler directives actually a problem? Remember that the usual approach is to let the configure script or CMake setup the correct preprocessing flags one the particular collection of compiler bugs have been identified.
> 4) Would there be any objections to refactoring the internal, private code so that all module functions and subroutines have names that start with the name of the module, I find this makes the code much more readable. Calls to overloaded routines can then be spotted
> easily by not conforming.
Not really, but there is a point where the length of function names becomes too long to read (especially where a function is called a lot) and a 31 character limit on Fortran function / variable names.
> 5) Can we review the extent to which recursive routines have been used because, under IVF and Windows atleast, one can run out of stack space very quickly.
FoX actually makes very little use of recursion (e.g. the DOM tree walking code is not recursive, the natural recursion scheme is expressed as a set of loops). The full list of recursive procedures is below. Although I've never measured it, I don't think FoX ever uses very much stack space.
It wouldn't surprise me if you are actually seeing a bug in the Intel Fortran compiler. (IVF is just iFort underneath, isn't it?) In particular, this sounds like issue DPD200161472 (the compiler fails to remove pointers to character arrays from the stack - see http://software.intel.com/en-us/forums/showthread.php?t=77847&o=a&s=lr - present since at least IVF 11.1.067). This was only fixed in the September 2011 release of the compiler (see http://software.intel.com/en-us/articles/intel-composer-xe-2011-compilers-fixes-list/ ). Is your compiler older than that?
Recursive procedures:
==================
andreww@boa:~/Code/FoX/fox$ grep recursive */*.f90 */*.F90
dom/m_dom_utils.f90: recursive subroutine dump2(input)
dom/m_dom_dom.F90: recursive subroutine setParameter(domConfig, name, value, ex)
dom/m_dom_dom.F90: recursive function getParameter(domConfig, name, ex)result(value)
dom/m_dom_dom.F90: recursive subroutine destroyNode(np, ex)
dom/m_dom_dom.F90: recursive subroutine destroyElementOrAttribute(np, ex) dom/m_dom_dom.F90: recursive subroutine destroyAllNodesRecursively(arg, except)
dom/m_dom_dom.F90: recursive function cloneNode(arg, deep, ex)result(np) dom/m_dom_dom.F90: ! Needs to be recursive in case of entity-references within each other.
dom/m_dom_dom.F90: recursive function isEqualNode(arg, other,
ex)result(p)
dom/m_dom_dom.F90: ! a pure function, we cant do the recursive walk we need to.
dom/m_dom_dom.F90: recursive function getNodePath(arg, ex)result(c) dom/m_dom_dom.F90: ! recursive only for atts and text
dom/m_dom_dom.F90: recursive function createEntityReference(arg, name, ex)result(np)
dom/m_dom_dom.F90: ! Needs to be recursive in case of entity-references within each other.
dom/m_dom_dom.F90: recursive subroutine namespaceFixup(this, deep, ex) sax/m_sax_parser.F90: recursive subroutine sax_parse(fx, fb, & sax/m_sax_tokenizer.F90: recursive function normalize_attribute_text(fx, s_in) result(s_out)
sax/m_sax_tokenizer.F90: recursive function expand_pe_text(fx, s_in, fb) result(s_out)
Cheers,
Andrew
>
> --
Andrew Walker
http://www1.gly.bris.ac.uk/~walker/
phone: 0117 9545698
Department of Earth Sciences,
University of Bristol,
Wills Memorial Building,
Queen’s Road,
Bristol, BS8 1RJ, UK
Norman Kirkby
Sorry for the delay replying. What I suggest is that a PhD student of mine, Tom Mee, and I take on some of this work. There will be a delay because Tom has to complete his transfer report first.
I think it would be a useful contribution to get out our instructions for use under windows but at present there appear to be a couple of bugs which prevent the FoX examples running.
I need to investigate further, as time permits, but there appears to be two consecutive deallocations of the same object - would this cause an error under Unix?
In the meanwhile, is there a Github project or similar that Tom and I have to join?
N
Sent from my iPad
> --
> You received this message because you are subscribed to the Google Groups "FoX-discuss" group.
> To post to this group, send email to fox-d...@googlegroups.com.
> To unsubscribe from this group, send email to fox-discuss...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/fox-discuss?hl=en.
>
Andrew Walker
On 18 Nov 2011, at 14:17, Norman Kirkby wrote:
> Sorry for the delay replying. What I suggest is that a PhD student of mine, Tom Mee, and I take on some of this work. There will be a delay because Tom has to complete his transfer report first.
Great.
> I think it would be a useful contribution to get out our instructions for use under windows but at present there appear to be a couple of bugs which prevent the FoX examples running.
OK.
> I need to investigate further, as time permits, but there appears to be two consecutive deallocations of the same object - would this cause an error under Unix?
Double deallocation is a bug that needs fixing but it is conceivable that it wouldn't actually cause a problem / crash. I've got memory checking tools (Valigrind) set up here, so if you could let me know which example seems to show this or send me some example code I should be able to track down (and hopefully fix) the problem without too much trouble.
> In the meanwhile, is there a Github project or similar that Tom and I have to join?
You don't need to join a project per se but should get a Github account. Then you just need to fork my repository (at https://github.com/andreww/fox - use the "Fork" button) and I'll be able to pull any changes you make and want including.
Cheers,
Andrew
--
Andrew Walker <andrew...@bris.ac.uk>