Skip to content

Make mse work for dense predictions - #26

Open
rcannood wants to merge 1 commit into
mainfrom
fix/mse-dense-predictions
Open

Make mse work for dense predictions#26
rcannood wants to merge 1 commit into
mainfrom
fix/mse-dense-predictions

Conversation

@rcannood

Copy link
Copy Markdown
Member

Describe your changes

file_prediction.yaml asks for a double normalized layer -- it says nothing about sparsity. Every method we ship happens to write a csc_matrix, so this has never bitten, but a method returning a dense array crashes the metric:

>>> (sol.layers["normalized"] - pred.layers["normalized"].toarray()).power(2)
AttributeError: 'matrix' object has no attribute 'power'

sparse - ndarray yields a np.matrix, which has no .power(). A contributor hitting this gets a crashed metric instead of a poor score, which is the wrong failure mode -- especially with errorStrategy = 'ignore' turning it into a silently missing row.

Coercing both layers to CSR first. Scores are unchanged for the sparse case:

sparse prediction: (1.2189312040309257, 0.9132953464791143)
dense  prediction: (1.2189312040309257, 0.9132953464791143)

Part of a series of PRs coming out of a pre-run review of the benchmark.

Checklist before requesting a review

  • I have performed a self-review of my code

  • Check the correct box. Does this PR contain:

    • Breaking changes
    • New functionality
    • Major changes
    • Minor changes
    • Bug fixes
  • Proposed changes are described in the CHANGELOG.md

  • CI Tests succeed and look good!

sparse - dense yields a np.matrix, which has no .power(), so a method
returning a dense normalized layer crashed the metric rather than
scoring poorly. Coerce both layers to csr first.
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

Successfully merging this pull request may close these issues.

1 participant