Skip to content

implemented multiclass for svc#308

Merged
Mec-iS merged 4 commits intosmartcorelib:developmentfrom
DanielLacina:svm_multiclass
Jun 16, 2025
Merged

implemented multiclass for svc#308
Mec-iS merged 4 commits intosmartcorelib:developmentfrom
DanielLacina:svm_multiclass

Conversation

@DanielLacina
Copy link
Copy Markdown
Contributor

No description provided.

@DanielLacina DanielLacina requested a review from Mec-iS as a code owner June 15, 2025 15:06
@DanielLacina
Copy link
Copy Markdown
Contributor Author

I messed up. My .git directory got corrupted and then I deleted it. Then I initialized a new .git folder and forced push the code but that erased the commit history. Im sorry about that.

@DanielLacina
Copy link
Copy Markdown
Contributor Author

DanielLacina commented Jun 15, 2025

impl<'a, TX: Number + RealNumber, TY: Number + Ord, X: Array2<TX>, Y: Array1<TY>>
    SupervisedEstimatorBorrow<'a, X, Y, SVCParameters<TX, TY, X, Y>> for SVC<'a, TX, TY, X, Y>
{
    fn new() -> Self {
        Self {
            classes: Option::None,
            instances: Option::None,
            parameters: Option::None,
            w: Option::None,
            b: Option::None,
            phantomdata: PhantomData,
        }
    }
    fn fit(
        x: &'a X,
        y: &'a Y,
        parameters: &'a SVCParameters<TX, TY, X, Y>,
    ) -> Result<Self, Failed> {
        SVC::fit(x, y, parameters)
    }
}```

@DanielLacina
Copy link
Copy Markdown
Contributor Author

Isnt this the API?

@DanielLacina
Copy link
Copy Markdown
Contributor Author

Anyways Ill fix it.

@DanielLacina
Copy link
Copy Markdown
Contributor Author

PR is ready for review.

@Mec-iS
Copy link
Copy Markdown
Collaborator

Mec-iS commented Jun 15, 2025

Good! all checks passed.

ok, this is better but you didn't need to open a new PR, see also -> #306 (comment)

This PR's code is closer to what I had in mind just: when you add a method to an existing interface, like multiclass_fit, it goes at the end of the list (not at the beginning) to not break the reading convention. Same for the tests, you don't add the top but at the bottom. I know this sounds like nitpicking but it is good code hygiene.

@DanielLacina
Copy link
Copy Markdown
Contributor Author

alright Ill change it accordingly

@DanielLacina
Copy link
Copy Markdown
Contributor Author

@Mec-iS fixed

@Mec-iS
Copy link
Copy Markdown
Collaborator

Mec-iS commented Jun 16, 2025

Good one 🚀 I will merge it when the checks have finished. It will be released with v0.4.2

Copy link
Copy Markdown
Collaborator

@Mec-iS Mec-iS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Mec-iS Mec-iS merged commit 730c0d6 into smartcorelib:development Jun 16, 2025
11 checks passed
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.

2 participants