Better names for `correlated_values` and `correlated_values_norm`

[jagerber48]

We have the functions covariance_matrix and correlation_matrix which are both pretty clear. correlated_values and correlated_values_norm are, by comparison, not symmetric with the "covariance"/"correlation" nomenclature and which are just not clear.

Recommendation: Rename this to ufloats_from_correlation_matrix and ufloats_from_covariance_matrix and maintain correlated_values and correlated_values_norm as legacy wrappers. If it was up to me alone I would say we deprecate those names over time and eventually remove them.

I could be convinced to use from_correlation_matrix or from_covariance_matrix as shorter names.

I don't think these changes need to happen before 4.0 because they don't involve breaking changes and I don't want to distract, but I think it would be nice.

[newville]

@jagerber48 Sorry, but what is not clear? correlated_values() has a signature and doc string of

def correlated_values(nom_values, covariance_mat, tags=None):
    """
    Return numbers with uncertainties (AffineScalarFunc objects)
    that correctly reproduce the given covariance matrix, and have
    the given (float) values as their nominal value.
    ....

That seems clear to me: return a set of ufloats that are correlated. As it turns out, we use this function in lmfit.

If you want a shorter name, I would suggest correlate().

[jagerber48]

@newville Yes, the docstring is clear, and yes, I agree this is a very useful and cool function. I would love if more was obvious from just the function name without even needing to go to the docstring.

The thing that really bothers me about the names is that I find it confusing that the correlated_values function accepts the covariance matrix, not the correlation matrix, as input since it has "correlated" in the name and nothing about "covariance". I would like to be obvious from the names for the two "construct UFloats from covariance/correlation matrices" functions to make it obvious which one wants the covariance matrix and which ones the correlation matrix. Also the name correlated_values_norm is a bit confusing. What is trying to be communicated is that "this is the version of correlated_values that wants the correlation matrix which is the normalized version of the covariance matrix, not the covariance matrix. Hence my suggestsions above.

---

Also I don't like the signature

def correlated_values_norm(values_with_std_dev, correlation_mat...)
    ...

I would prefer

def ufloats_from_correlation_matrix(nominal_values, std_devs, correlation_mat...)
    ...

# or

def ufloats_from_corr_mat(nominal_values, std_devs, correlation_mat...)
    ...

---

Overall this might be overstepping with regards to wanting the API to be a specific way, however, I do consider the covariance/correlation confusion to be an actual issue that warrants some discussion. This is a low priority issue. I would like to see better names but I don't want to waste anyone's time on it...

[newville]

@newville I would give a solid "Yuck" to ufloats_from_correlation_matrix or anything like thing_from_other_thing_with_a_long_name. Objects are things/nouns. Functions are actions/verbs.

If changing the name of this function for the sake of changing the name, I think it should be:

def correlate(values, covar=None, correl=None):

This function correlates a sequence of values (nominal values) using a covariance matrix. Or, a correlation matrix.

Let's not make a despicable API. And as you might suspect, by despicable, I do indeed mean Java.

FWIW, this sort of makes me lean more to ufloat() over UFloat(). It is fine for a function (lower case) to create and return an object. Functions that return None (or a cached object like a small integer) don't return created objects, but all other functions do. Like, why not name the class ufloat?

[jagerber48]

correlate() is acceptable to me, but the API switching between correlation and covariance matrices is tricky. What if users passing something in for both covar and correl? Also remember that for the correl option users must also pass std_devs.

def correlate_covar_mat(values, covar_mat):
    ...

def correlate_corr_mat(values, std_devs, corr_mat):
    ....

???

I can't think of a non-ugly signature that combines these two:

def correlate(values, covar_mat=None, corr_mat=None, std_devs=None):
    ...

The user should either

• Pass something in for covar_mat but nothing for corr_mat and std_devs or
• Pass something in for corr_mat and std_devs but not covar_mat

If they do anything else we need to figure out how to handle it. Some cases are necessarily exceptions but others aren't. Do we strictly raise exceptions? Do we warn? Silently choose a resolution? I find functions that switch behavior based on the types of their inputs to be confusing and a code smell.

[newville]

@jagerber48 I would just go with

def correlate(values, covar):
    "Return a tuple of correlated values from a list/tuple of nominal values and a covariance matrix"
    # calls ucorrelate() with properly constructed uvalues and normalized correlation matrix

def ucorrelate(uvalues, correl):
   "Return a tuple of correlated values from a list/tuple of ufloats and a correlation matrix"

FWIW, I would suggest dropping "tags" and enforcing the output to be in the order of the input. It's OK to leave it, but if asked, I would be "-1".