public inbox for gsl-discuss@sourceware.org
 help / color / mirror / Atom feed
From: Rhys Ulerich <rhys.ulerich@gmail.com>
To: timflutre@gmail.com
Cc: gsl-discuss@sourceware.org
Subject: Re: [Help-gsl] Spearman rank correlation coefficient
Date: Fri, 02 Mar 2012 15:59:00 -0000	[thread overview]
Message-ID: <CAKDqugT=cGUTGg2T4HEXrecTC__aLTYR6y_K07nobGuHDvOsTg@mail.gmail.com> (raw)
In-Reply-To: <CAGJVmuKKNcAC5DkdtMSUfd7BLSYWDfQK5wW12H5Sb8nU_bGMTw@mail.gmail.com>

Hi Tim,

> a month ago I proposed an implementation of the Spearman rank
> correlation coefficient ...
> https://gist.github.com/1784199#file_spearman_v2.c
>
> ... Thus can I submit it to the GSL main trunk?
> I've never done that before. Can someone indicate me what to do?
> Should I request "developer write access" for instance?

From reviewing the code at the github...

Your gsl_stats_spearman_workspace (e.g.) should be passed into your
gsl_stats_spearman method.
You'll want to declare a struct-based workspace rather than
allocate/deallocate many separate variables in your
gsl_stats_spearman_alloc/gsl_stats_spearman_free.  As an example,
check out gsl_bspline_workspace from bspline/gsl_bspline.h which is
allocated/deallocated by gsl_bspline_alloc/gsl_bspline_free.  You'll
want to add the allocation-failure error handling as in the bspline
example.

You gsl_stats_spearman_rank implementation should always be checking
if calloc/realloc failed and handle that error appropriately

Helper functions like "resolveTies" which should not be publicly
visible should be declared static.  That way they can only be called
from within the same file.

Finally, the Apache License 2.0 origin of the logic should be made
more clear in the file.  Just attributing the logic with "Source"
seems insufficient-- I'd organize the file so everything below some
point if the Apache-derived code and everything above that point is
your GSL-ready GPL3 logic.

Give a shout when you're ready and I'll take another look.  I can help
with getting the final logic into GSL's trunk.

- Rhys

  reply	other threads:[~2012-03-02 15:59 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAGJVmuKKL4HCXViqxAf3b=em05hb7ox5pABm5b_LdKNMxTLMkg@mail.gmail.com>
2012-02-10  0:05 ` Patrick Alken
2012-02-11 20:53   ` Timothée Flutre
2012-03-02 15:33     ` Timothée Flutre
2012-03-02 15:59       ` Rhys Ulerich [this message]
2012-03-02 16:01       ` Patrick Alken

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKDqugT=cGUTGg2T4HEXrecTC__aLTYR6y_K07nobGuHDvOsTg@mail.gmail.com' \
    --to=rhys.ulerich@gmail.com \
    --cc=gsl-discuss@sourceware.org \
    --cc=timflutre@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).