<p>Ignoring the zeros is pretty baked into the CF/AMF code right now. Here are a few instances I found when I was looking at the code:</p>

<p><a href="https://github.com/mlpack/mlpack/blob/660fd96eecf88245555a8651e1cbeff50cd66686/src/mlpack/methods/cf/cf_impl.hpp#L170">https://github.com/mlpack/mlpack/blob/660fd96eecf88245555a8651e1cbeff50cd66686/src/mlpack/methods/cf/cf_impl.hpp#L170</a><br>
<a href="https://github.com/mlpack/mlpack/blob/660fd96eecf88245555a8651e1cbeff50cd66686/src/mlpack/methods/amf/init_rules/average_init.hpp#L42">https://github.com/mlpack/mlpack/blob/660fd96eecf88245555a8651e1cbeff50cd66686/src/mlpack/methods/amf/init_rules/average_init.hpp#L42</a><br>
<a href="https://github.com/mlpack/mlpack/blob/660fd96eecf88245555a8651e1cbeff50cd66686/src/mlpack/methods/amf/update_rules/svd_batch_learning.hpp#L91">https://github.com/mlpack/mlpack/blob/660fd96eecf88245555a8651e1cbeff50cd66686/src/mlpack/methods/amf/update_rules/svd_batch_learning.hpp#L91</a><br>
<a href="https://github.com/mlpack/mlpack/blob/660fd96eecf88245555a8651e1cbeff50cd66686/src/mlpack/methods/amf/update_rules/svd_complete_incremental_learning.hpp#L84">https://github.com/mlpack/mlpack/blob/660fd96eecf88245555a8651e1cbeff50cd66686/src/mlpack/methods/amf/update_rules/svd_complete_incremental_learning.hpp#L84</a></p>

<p>Hence the pull request. I did see the <code>sp_mat</code> constructor taking a <code>check_for_zeros</code> parameter. However, the comment in the armadillo documentation states (<a href="http://arma.sourceforge.net/docs.html#SpMat">http://arma.sourceforge.net/docs.html#SpMat</a>)</p>

<pre><code>If check_for_zeros is set to false, the values vector is assumed to contain no zero values
</code></pre>

<p>As an API user, when I read something like this, it suggests to me that putting explicit zero indices in is incorrect. Whether or not it actually is, I assume you are more familiar with since I'm guessing you wrote some of the sparse matrix code? The point is, to set <code>check_for_zeros = false</code>, and then explicitly stuff zero values in, seems to break the assumption of the parameter. </p>

<p>I'm not that opposed to the idea, just wanted to point out some discrepancies before proceeding. </p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">&mdash;<br>Reply to this email directly or <a href="https://github.com/mlpack/mlpack/pull/376#issuecomment-69620397">view it on GitHub</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/AJ4bFH_vGMAJXpeUv7Rxl33lS4rC6uKpks5nhAvXgaJpZM4DQ8wr.gif" width="1" /></p>
<div itemscope itemtype="http://schema.org/EmailMessage">
  <div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
    <link itemprop="url" href="https://github.com/mlpack/mlpack/pull/376#issuecomment-69620397"></link>
    <meta itemprop="name" content="View Pull Request"></meta>
  </div>
  <meta itemprop="description" content="View this Pull Request on GitHub"></meta>
</div>