Add contribution guide. #309
Conversation
Should help merging PR without introducing issues
|
any thought @iejMac @MattUnderscoreZhang ? |
|
Yeah this sounds great. The 14.4 videos/s/core probably depends on your system though. I think it may be better to specify that the number on your system should be identical before and after your commit. I also think running just a subset of results_2M_val should be default, if your only intention is to check the speed. Otherwise the comparison would take many hours for each commit. |
|
It may also be good to specify what kind of environment to test on. For example, one with at least 16 cores. And it would be difficult for most people to test that e.g. Slurm is still working correctly. |
|
Unfortunately it's hard to specify a general test, because sometimes you also want to check that whatever subsampler you touched didn't reduce the speed. The webvid config specified basically only does video download, but e.g. if you touch the resolution subsampler you'd want to test with a config that included resolution downscaling in its processing pipeline. Thus it may make sense to test with your specific use case, as I did. Doing so would make the 14.4 videos/s/core benchmark even less relevant. |
|
Yes we can add these details. The correct way to address this is many automated tests, see #310 But a manual test of that kind is better than nothing |
I bet it doesn't depend that much. It may vary .5-2x but not much more |
|
Is it possible to set up a runner on an environment you guys have access to? You'd have to be ok with it running each time someone does a pull request, but it would make it easier to compare numbers on a standard setup. |
Should help merging PR without introducing issues