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

MatrixFactorization construction parameters are not consistent with other learners #1826

Open
singlis opened this Issue Dec 5, 2018 · 0 comments

Comments

Projects
1 participant
@singlis
Member

singlis commented Dec 5, 2018

The ordering of parameters for the Matrix Factorization constructor is inconsistent with other trainers, specifically the Label column should be first before the matrixColumnIndex and matrixRowIndex as the matrixColumnIndex and matrixRowIndex are the feature columns:

        /// <summary>
        /// Initializing a new instance of <see cref="MatrixFactorizationTrainer"/>.
        /// </summary>
        /// <param name="env">The private instance of <see cref="IHostEnvironment"/>.</param>
        /// <param name="matrixColumnIndexColumnName">The name of the column hosting the matrix's column IDs.</param>
        /// <param name="matrixRowIndexColumnName">The name of the column hosting the matrix's row IDs.</param>
        /// <param name="labelColumn">The name of the label column.</param>
        /// <param name="advancedSettings">A delegate to apply all the advanced arguments to the algorithm.</param>
        public MatrixFactorizationTrainer(IHostEnvironment env,
            string matrixColumnIndexColumnName,
            string matrixRowIndexColumnName,
            string labelColumn = DefaultColumnNames.Label,
            Action<Arguments> advancedSettings = null)
            : base(env, LoadNameValue)

Where other trainers have the label column argument followed by feature column:

 public static SdcaBinaryTrainer StochasticDualCoordinateAscent(
                this BinaryClassificationContext.BinaryClassificationTrainers ctx,
                string labelColumn = DefaultColumnNames.Label,
                string featureColumn = DefaultColumnNames.Features,
                string weights = null,
...)

@singlis singlis added the bug label Dec 5, 2018

@sfilipi sfilipi added this to To do in Project 13 via automation Dec 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment