From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27244 invoked by alias); 19 Feb 2010 00:00:19 -0000 Received: (qmail 27229 invoked by uid 22791); 19 Feb 2010 00:00:16 -0000 X-SWARE-Spam-Status: No, hits=-9.3 required=5.0 tests=AWL,BAYES_20,RCVD_IN_DNSWL_HI,SPF_PASS X-Spam-Check-By: sourceware.org Received: from proofpoint2.lanl.gov (HELO proofpoint2.lanl.gov) (204.121.3.26) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 19 Feb 2010 00:00:10 +0000 Received: from mailrelay1.lanl.gov (mailrelay1.lanl.gov [128.165.4.101]) by proofpoint2.lanl.gov (8.14.3/8.14.3) with ESMTP id o1INxwSS017607; Thu, 18 Feb 2010 16:59:59 -0700 Received: from alvie-mail.lanl.gov (alvie-mail.lanl.gov [128.165.4.110]) by mailrelay1.lanl.gov (Postfix) with ESMTP id DEC32241637; Thu, 18 Feb 2010 16:59:58 -0700 (MST) Received: from localhost (localhost.localdomain [127.0.0.1]) by alvie-mail.lanl.gov (Postfix) with ESMTP id DBD0D7D0044; Thu, 18 Feb 2010 16:59:58 -0700 (MST) X-NIE-2-Virus-Scanner: amavisd-new at alvie-mail.lanl.gov Received: from [130.55.124.157] (manticore.lanl.gov [130.55.124.157]) by alvie-mail.lanl.gov (Postfix) with ESMTP id C07BD7D0042; Thu, 18 Feb 2010 16:59:58 -0700 (MST) Subject: Re: Feedback from GSL folks on libflame 4.0 From: Gerard Jungman To: "Field G. Van Zee" Cc: gsl-discuss mailing list In-Reply-To: <4B7D90B5.4020707@cs.utexas.edu> References: <4a00655d1002171047t4e87fb85w88b609245e3f9a8e@mail.gmail.com> <4B7D90B5.4020707@cs.utexas.edu> Content-Type: text/plain Date: Fri, 19 Feb 2010 00:00:00 -0000 Message-Id: <1266537635.27033.79.camel@manticore.lanl.gov> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-Proofpoint-Virus-Version: vendor=fsecure engine=1.12.8161:2.4.5,1.2.40,4.0.166 definitions=2010-02-18_14:2010-02-06,2010-02-18,2010-02-18 signatures=0 Mailing-List: contact gsl-discuss-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gsl-discuss-owner@sourceware.org X-SW-Source: 2010-q1/txt/msg00028.txt.bz2 On Thu, 2010-02-18 at 13:10 -0600, Field G. Van Zee wrote: > Thanks for taking the time to review our library! I'll try to respond to each item. Hi. This is great, someone to talk to! Thanks very much for replying. Here are my follow-on comments. > The --enable-multithreading option enables multithreading within the library > only. It does not change the friendliness of the library routines to being > invoked from multiple threads created outside of libflame. (I'm not sure if > libflame would work in such a situation.) Granted, this is not what we want, but > making the library thread-safe/reentrant/etc, has not been a high priority in > the past. It might be re-entrant anyway, or so close to it that only minor changes would be needed. The global constants are not a problem, in principle. The memory leak counter may have to be initialized on a per-thread basis, if that makes sense, similar to the way errno is made thread-safe in gcc. Brian might be able to look into this, since he understands how errno works. The trick would be doing it in a platform-agnostic way, if not truly platform-independent. In the final picture, I think thread-safety is really important. More and more, it's going to become a requirement for new code. Of course, people will get by however they can, but it would be nice if libflame just worked for them, in those cases where some linear algebra operation solves their problem. > We also have plans to add native implementations > of EVD/SVD in the future, but that is probably still 6-9 months away. That sounds fine. After all, some of us have been waiting a decade already. We can wait another year. > Could you clarify what you mean by "stack friendly"? We declare FLA_Obj's on the > stack, but they still need to be initialized and have data buffers "attached" to > them. Both of these things happen when the object is created via one of the > FLA_Obj_create_*() routines. I have looked at the code now. The "void* buffer" element of FLA_Base_Obj is not a problem, since it can be attached by the client to anything appropriate, as you say. In fact, FLA_Base_Obj is stack-friendly in the sense that I mean, as it does not require any heap operations to put it into a well-defined state. However, it seems that FLA_Obj is not. Of course, this is not an absolute show-stopper, but it's still a bit sticky, since it touches on your object model. FLA_Obj_create() eventually does a heap allocation to assign the "FLA_Base_obj* base" member of FLA_Obj. From my point of view, this heap allocation is gratuitous. I don't think this is a trivial point, because simple workarounds (like providing another way to attach "FLA_Base_obj* base" might open holes in the safety of the whole system. One way is to use a kind of composition model, where the view carries FLA_base_Obj directly, rather than by pointer reference. But this requires rethinking your object model. Also, the simple question of memory overhead arises, in the case when many views to a small number of underlying objects are created. We had this same discussion on the GSL list recently, regarding a possible re-design of the GLS containers. The current GSL containers have the same problem (along with other problems!). I tried hard there to make the object/view paradigm work well without large overheads. Of course, that discussion is still in flux. > We agree that > using return values would be a more standard way of handling errors, but we're > also somewhat cynical in that we don't trust our users to check return values. I agree with the cynicism. GSL tried to handle this by providing "natural" versions of functions (which act like yours, aborting when they give up) and "return-code" versions which are meant for people who want them, and especially for external library designers who want to use GSL components but need to handle errors in their own way. This includes situations like C++ library designers who want to wrap GSL but want to throw exceptions, etc. The "natural" versions defer to the "return-code" versions in the obvious way. Of course, this is only one possible solution. There are others, up to and including a full-blown callback-based system (which I am not advocating). > Bottom line: we are anything but married to the idea of aborting when an error > is encountered, but we are unsure how to come up with a solution that is less > brain-damaged and portable and that fits our users' coding style. It would be > nice to be able to throw exceptions, but of course C has no such mechanism. I would seriously consider the two-prototype solution. I don't think we have discovered any real problems with it. It increases the size of the intellectual real-estate occupied by the API, but people can ignore the parts they don't need. > Many of the options don't need to be tweaked, and/or can be "fixed" to their > more portable or more general setting. As for choosing a different build at > link-time, I can't really offer much in the way of comment there. Sure. This was not meant so much for you, as for GSL folks, asking how we would support variability in this configuration layer, when using libflame through some link-time selection process. The desire is that the variations could be handled without client-side source-code changes. I assume and hope that this is possible. > We have tentative plans to integrate a BLAS implementation at some point in the > future. At that point, libflame will be a complete solution that does not depend > on any external numerical library (aside from m). But yes, for now, we require > external BLAS. Right. This will be a good thing eventually. Whatever you produce will end up being much better than the gslcblas implementation, which I basically copied from the CBLAS draft standard, oh so many years ago. Of course, people will still want to use (and should want to use) their favorite optimized BLAS, but we could eliminate an entire chunk of annoying code from GSL. > See above discussion of (1). If we were ever to remove the global scalars, we > might need to replace them with preprocessor macros or perform some other kind > of magic. Alternatively, we could create and free these scalar objects every > time we needed them, though I cringe at the idea of implementing that change. > Any suggestions? I probably can't suggest anything that you haven't already thought of. And I can't really judge without having tried to use the stuff. But I do wonder if the logical coherence of the "scalar = 1x1 object" choice is not offset by the possible client inconvenience. And that's not to say that there isn't some magical solution waiting to be found. But I don't know one off hand. > > (d) There are several places where the API assumes C stdio. It looks > > like some of these uses are internal (like FLA_Print_message > > being used for error messages). This is brain-damaged, since > > it makes it harder to integrate into other environments > > (i.e. C++) where C stdio is not appropriate. It's ok to > > have such "convenience" functions in the API, but they > > should not be used internally. > > Please suggest a fix and we'll be happy to look into it! With regard to the error messages, that will be fixed by whatever error-handling solution is chosen. Printing the error messages only makes sense for the "abort" versions of functions anyway. I don't know if FLA_Print_message() is used anywhere else. Maybe for "informational" or "warning" messages now and then, but this is also handled by whatever solution is chosen for error-handling. I can't imagine C stdio appears anyplace else. Is that right? > Notice that we only use autoconf, not automake or libtool. Why is using autoconf > undesirable? We were trying to be good GNU software citizens when we designed > the build system. I think I'm glad that automake and libtool are not being used. I guess that I'm mostly agnostic about autoconf on its own. Others may have specific complaints that they want to air. By the way, how do you build shared libraries? This comment is partly my way of jabbing at Brian regarding the GSL build system. These days I am following a new herd and converting all my own projects to cmake. Ten years from now I will probably be complaining about cmake. > The naming conventions were settled upon before I joined the group. I suspect we > were trying to mimic MPI and other such libraries. It's not exactly my idea > naming convention. I prefer all lower-case with underscores. Who knows, maybe we > will change it at some point in the future? Yup. That's my preference too, all lower-case with underscores. > Don't worry about appearing critical. We appreciate all your feedback! Hopefully > we can make improvements on our side that will make your life easier. And I very much appreciate your responses. Feel free to straighten out any misconceptions I have. All these comments are based on an afternoon examining the manual and some of the code, and not real usage, so they should be taken as highly provisional. Thanks again! -- G. Jungman