search

microsoft / knossos-ksc

Compiler with automatic differentiation
Other
45 stars 10 forks source link

Run benchmark on PR but don't upload to Azure #862

Closed cgravill closed 3 years ago

cgravill commented 3 years ago

Context here: https://github.com/microsoft/knossos-ksc/pull/855#issuecomment-856900161 where the nightly benchmark would have failed.

Still uploads to the artifacts.

We don't upload to Azure off master, we could and just filter but for now this added a correctness test without complicating the comparison.

dcrc2 commented 3 years ago

I definitely agree that this is a good idea.

I'm not sure what will happen if this is merged with #855: the vmap work requires a recent build of PyTorch, which I don't think our CI is using. So I'm expecting it to fail, though I may be wrong!

cgravill commented 3 years ago

I definitely agree that this is a good idea.

I'm not sure what will happen if this is merged with #855: the vmap work requires a recent build of PyTorch, which I don't think our CI is using. So I'm expecting it to fail, though I may be wrong!

Failing before PR merge seems like a good thing, more information at least! Then we'll either have to switch to a preview version of PyTorch or selectively disable some benchmarks.

The sqrl benchmark doesn't use vmap though so far, so will this be an issue yet?

dcrc2 commented 3 years ago

The sqrl benchmark doesn't use vmap though so far

Good point! It seems we don't refer to torch.vmap unless the example being tested contains that, so it should be fine.