Rhys Ulerich | 1 Jun 2012 18:31
Picon
Gravatar

Need input on inconsistency in gsl_ieee_env_setup docs

This line from the gsl_ieee_env_setup() documentation
(http://www.gnu.org/software/gsl/manual/html_node/Setting-up-your-IEEE-environment.html)
makes no sense

> If the requested modes are not supported by the platform
> being used then the function calls the error handler and
> returns an error code of GSL_EUNSUP.

as the relevant function signature is

> void gsl_ieee_env_setup (void)

The GSL_EUNSUP value is returned from gsl_ieee_set_mode within
gsl_ieee_env_setup's implementation, but the value is ignored and not
returned to the caller.

So:

a) Fix the documentation.  Easy.

b) Fix the API signature and implementation.  Presumably everyone's
ignoring the currently void return value anyhow and changing the
signature to return an int would harm no one.

Thoughts?

- Rhys

Rhys Ulerich | 2 Jun 2012 15:34
Picon
Gravatar

Re: [Help-gsl] possible bug for vectors and matrices

Hi Jerome (and bug-gsl added on CC),

> I have just gone through some gsl_(vector|matrix)_TYPE.h headers (with
> TYPE=long, long_double, ...). Functions that scale by or add a
> constant has a *double* as constant argument: should this argument
> constant not be in type TYPE rather than in type double?

Just to be concrete, you're saying all of

int gsl_vector_long_double_scale (gsl_vector_long_double * a, const double x);
int gsl_vector_long_double_add_constant (gsl_vector_long_double * a,
const double x);
int gsl_matrix_long_double_scale (gsl_matrix_long_double * a, const double x);
int gsl_matrix_long_double_add_constant (gsl_matrix_long_double * a,
const double x);
int gsl_matrix_long_double_add_diagonal (gsl_matrix_long_double * a,
const double x);

should be taking long doubles as their second argument?  Likewise for
other types?  I agree.

For anything "smaller" than double I doubt that the bug will cause a
loss of precision.  But for long doubles and above it is indeed a
problem.  Either way, the API isn't right.

Anyone have an objection if I throw this in a ticket on savannah and
take a stab at fixing it by correcting the templating?  (I think the
right template macro is BASE rather than TYPE, but that's an
implementation detail).

(Continue reading)

Rhys Ulerich | 2 Jun 2012 19:35
Picon

[bug #36197] reserved identifier violation

Follow-up Comment #1, bug #36197 (project gsl):

I agree that "__GSL_MATH_H_" is problematic according to the link you sent. 
That fix is just a judicious 'perl -pi -e' invocation across the code base and
should be doable.

At a glance, I'm uncertain why "gsl_mode_t" and "gsl_prec_t" tamper with the
POSIX name space.  I see the "ANY header" table entry in the specification you
linked.  It seems the clause "End in the string indicated as a reserved suffix
in the table and do not use the reserved prefixes posix_, POSIX_, or _POSIX_,
as long as the reserved suffix is in that part of the name considered
significant by the implementation." may prevent "gsl_mode_t" from being
problematic, e.g., because it does not have a posix-y prefix.

Regardless, changing the GSL types ending with a "_t" would break the existing
public APIs.  I don't think this part of your report is fixable.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?36197>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/

Rhys Ulerich | 2 Jun 2012 19:39
Picon

[bug #36577] vector/matrix _add, _scale routines take wrong scalar type

URL:
  <http://savannah.gnu.org/bugs/?36577>

                 Summary: vector/matrix _add,_scale routines take wrong scalar
type
                 Project: GNU Scientific Library
            Submitted by: rhysu
            Submitted on: Sat 02 Jun 2012 05:39:26 PM GMT
                Category: None
                Severity: 3 - Normal
        Operating System: 
                  Status: Confirmed
             Assigned to: rhysu
             Open/Closed: Open
                 Release: 
         Discussion Lock: Any

    _______________________________________________________

Details:

Report sent to help-gsl and CCed to bug-gsl.
---8<---
Hello List:

I have just gone through some gsl_(vector|matrix)_TYPE.h headers (with 
TYPE=long, long_double, ...).
Functions that scale by or add a constant has a *double* as constant
argument:
should this argument constant not be in type TYPE rather than in type double
(Continue reading)

Rhys Ulerich | 2 Jun 2012 19:41
Picon

[bug #36578] inconsistency in gsl_ieee_env_setup doc/api

URL:
  <http://savannah.gnu.org/bugs/?36578>

                 Summary: inconsistency in gsl_ieee_env_setup doc/api
                 Project: GNU Scientific Library
            Submitted by: rhysu
            Submitted on: Sat 02 Jun 2012 05:41:51 PM GMT
                Category: Documentation
                Severity: 3 - Normal
        Operating System: 
                  Status: None
             Assigned to: rhysu
             Open/Closed: Open
                 Release: 
         Discussion Lock: Any

    _______________________________________________________

Details:

---8<---
This line from the gsl_ieee_env_setup() documentation
(http://www.gnu.org/software/gsl/manual/html_node/Setting-up-your-IEEE-environment.html)
makes no sense

> If the requested modes are not supported by the platform
> being used then the function calls the error handler and
> returns an error code of GSL_EUNSUP.

as the relevant function signature is
(Continue reading)

Markus Elfring | 2 Jun 2012 23:10
Picon

[bug #36197] reserved identifier violation

Follow-up Comment #2, bug #36197 (project gsl):

I suggest to be more careful with the use of double underscores for
identifiers
<http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier#228797>
in your source files.

Would you also like to clarify the interpretation of the word "and
<http://stackoverflow.com/questions/10356626/c-type-naming-conventions-t-or-allcaps>"
in the referenced paragraph?

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?36197>

_______________________________________________
  Nachricht gesendet von/durch Savannah
  http://savannah.gnu.org/

Rhys Ulerich | 3 Jun 2012 03:19
Picon

[bug #36197] reserved identifier violation

Follow-up Comment #3, bug #36197 (project gsl):

Thanks for the clarification.

So, the double include #defines can be fixed.  Changing the long-standing
public APIs isn't an option as it would break existing users.  Unfortunately
it's an old set of decisions that are irreversible at this time.  

Unless you object, I'd like to consider "fixing" this bug to be simply
changing "__GSL" to "GSL" in the double inclusion guards.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?36197>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/

Markus Elfring | 3 Jun 2012 10:44
Picon

[bug #36197] reserved identifier violation

Follow-up Comment #4, bug #36197 (project gsl):

How do you think about to append a kind of UUID
<http://en.wikipedia.org/wiki/Universally_unique_identifier> to your include
guards so that the probability for name clashes
<http://en.wikipedia.org/wiki/Include_guard#Difficulties> can be reduced a bit
more?

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?36197>

_______________________________________________
  Nachricht gesendet von/durch Savannah
  http://savannah.gnu.org/

Rhys Ulerich | 3 Jun 2012 11:14
Picon

[bug #36197] reserved identifier violation

Follow-up Comment #5, bug #36197 (project gsl):

Appending a UUID would definitely reduce the probability of name clashes. 
However, we don't seem to have a name clash problem given that there are no
outstanding bugs from name clashes and GSL is more than 16 years old.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?36197>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/

Rhys Ulerich | 3 Jun 2012 11:44
Picon
Gravatar

Re: Need input on inconsistency in gsl_ieee_env_setup docs

> b) Fix the API signature and implementation.  Presumably everyone's
> ignoring the currently void return value anyhow and changing the
> signature to return an int would harm no one.

The relevant functions returning errors to gsl_ieee_env_setup are
gsl_ieee_read_mode_string and gsl_ieee_set_mode.  Currently, if the
former returns an error the latter is still attempted after the error
handler is called.  With the default error handler, of course, the
program would exit after a gsl_ieee_read_mode_string failure.  But
with a user-provided error handler, it might not.  Fixing the API
signature /and/ returning the result of gsl_ieee_read_mode_string
without calling gsl_ieee_set_mode may quietly break some existing
code.  Fixing the API signature and returning only value of
gsl_ieee_set_mode feels wrong.

To me, it appears the better choice is

> a) Fix the documentation.

unless someone feels strongly about the issue.

- Rhys


Gmane