Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove redundant type casting, particularly for np.asarray #124

Open
bobleesj opened this issue Oct 29, 2024 · 3 comments
Open

Remove redundant type casting, particularly for np.asarray #124

bobleesj opened this issue Oct 29, 2024 · 3 comments

Comments

@bobleesj
Copy link
Contributor

For example in optimizers.py

def get_weights(stretched_component_gram_matrix, linear_coefficient, lower_bound, upper_bound):
    """Finds the weights of stretched component signals under a two-sided constraint

    Solves min J(y) = (linear_coefficient)' * y + (1/2) * y' * (quadratic coefficient) * y where
    lower_bound <= y <= upper_bound and stretched_component_gram_matrix is symmetric positive definite.
    Finds the weightings of stretched component signals under a two-sided constraint.

    Parameters
    ----------
    stretched_component_gram_matrix: 2d array like
      The Gram matrix constructed from the stretched component matrix. It is a square positive definite matrix.
      It has dimensions C x C where C is the number of component signals. Must be symmetric positive definite.

    linear_coefficient: 1d array like
      The vector containing the product of the stretched component matrix and the transpose of the observed
      data matrix. Has length C.

    lower_bound: 1d array like
      The lower bound on the values of the output weights. Has the same dimensions of the function output. Each
      element in 'lower_bound' determines the minimum value the corresponding element in the function output may
      take.

    upper_bound: 1d array like
      The upper bound on the values of the output weights. Has the same dimensions of the function output. Each
      element in 'upper_bound' determines the maximum value the corresponding element in the function output may
      take.

    Returns
    -------
    1d array like
      The vector containing the weightings of the components needed to reconstruct a given input signal from the
      input set. Has length C

    """

    stretched_component_gram_matrix = np.asarray(stretched_component_gram_matrix)
    linear_coefficient = np.asarray(linear_coefficient)
    upper_bound = np.asarray(upper_bound)
    lower_bound = np.asarray(lower_bound)

Originally posted by @sbillinge in #120 (comment)

@sbillinge
Copy link
Contributor

WE may want to think about using explicit typing too. We never adopted that till now, but it is widely used in the python world and can lead to better code (if less readable imho)

@bobleesj
Copy link
Contributor Author

I will try this in this package - it will also help minimize human efforts for writing types in docstrings since they will be automatically inferred

@sbillinge
Copy link
Contributor

this can be a new group standard if we like it. It makes apis, docstrings etc. better and avoids this typecasting issues. I don't think there are many drawbacks except for more typing at coding time. But it also forces us to think harder about what types we want (and why) which can also help the process. Let's try it on this package, but don't feel the need to do it for everything in the package (I am not sure how many functions there are).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants