search

dotnet / machinelearning

ML.NET is an open source and cross-platform machine learning framework for .NET.
https://dot.net/ml
MIT License
9.03k stars 1.88k forks source link

Get tests passing on Mono and add Mono to CI #2474

Open danmoseley opened 5 years ago

danmoseley commented 5 years ago

@Anipik and others have been scouting Mono.

As soon as our tests pass on Mono, we should add it to CI to protect it.

Note - the Mono build will need (at least) the fix for https://github.com/mono/mono/issues/12690

danmoseley commented 5 years ago

Assigning to @Anipik as he is scouting this.

Anipik commented 5 years ago

@eerhardt this is the sample code for the monoCI leg https://github.com/Anipik/machinelearning/commit/5579c80e8b954c34f55440088449157fd5d327c6

danmoseley commented 5 years ago

@eerhardt with respect to adding this to CI - as an alternative, perhaps we could do it in a rolling build run, that would not block PR's? We do need some kind of regular validation beyond @Anipik 's machine, or in my experience things break.

eerhardt commented 5 years ago

IMO that would actually be worse because then it would get broken, and no one would notice or fix it.

If you feel this is a need, then I have no objection to adding it in the CI. My questioning is whether this is a need or not.

danmoseley commented 5 years ago

My questioning is whether this is a need or not.

If we will declare that we broadly support use of 1.0 in Unity, which is a 1.0 goal, then we presumably want to protect the tests on Mono as a (poor) proxy. Either we test it "out of CI" (manually or automatically) which would get broken as you say, or we test it "in CI" as this suggests. Perhaps you're suggesting it's a sufficiently poor proxy for Unity that it isn't worth bothering? Maybe that's true I don't know.

I suggest we do this and we discover it's fragile then we can take action (like remove it while we make it not fragile, or defer doing that)

Anipik commented 5 years ago

Currently this task is blocked on https://github.com/mono/mono/issues/13113 (just linking it with the issue)

danmoseley commented 5 years ago

@eerhardt is of course right that if this isn't necessary to ship 1.0, we should defer it. Let's see what results you get.