Re: [Bf-committers] Mantaflow landing and unit tests.
Hi, There seems to be deeper issues than setting up new fluid scene for the regression test, and its solution was dragging for too long. So I have disabled the test which was failing. Next week when Sebastian is back to the office we will see if we can support fluid velocity for 2.82. On Fri, Jan 3, 2020 at 7:27 PM Ray Molenkamp wrote: > > One cruel thing i can think about is to force everyone to pay more > > attention is to prevent new build from being uploaded if regression tests > > are failing. We'll hear very quickly from users that there are no new > > builds on buildbot. > Cruel or not, It'll cost less resources to deal with > a single well defined test case, rather than triaging > and managing an unknown number of tickets that may > come from the same issue. I don't mind this idea *at all* > > --Ray > > ___ > Bf-committers mailing list > Bf-committers@blender.org > https://lists.blender.org/mailman/listinfo/bf-committers > -- With best regards, Sergey Sharybin ___ Bf-committers mailing list Bf-committers@blender.org https://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] Mantaflow landing and unit tests.
> One cruel thing i can think about is to force everyone to pay more > attention is to prevent new build from being uploaded if regression tests > are failing. We'll hear very quickly from users that there are no new > builds on buildbot. Cruel or not, It'll cost less resources to deal with a single well defined test case, rather than triaging and managing an unknown number of tickets that may come from the same issue. I don't mind this idea *at all* --Ray ___ Bf-committers mailing list Bf-committers@blender.org https://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] Mantaflow landing and unit tests.
Hi, > how can we do better Take responsibility for changes you do and get rid of "it's other's module problem" would be a good start. Not ignoring communication in this mailing list which doesn't include name of your project would be a good continuation. Making sure that a new project is added to the Release Notes should even come without reminding. The issue is not in Cycles and anyone familiar with physics/baking should be able to make required changes to the tests suite (bake and update reference image in this case). If you have tried to follow suggested solution (which is, re-bake) you would see how many bugs can be found in doing such a simple operation, and why it's taking so long for Cycles team to make needed adjustments. > We can take human element out of it, running the tests on a proposed patch While i agree we should cover patches with CI to test for regressions and code style, in this specific case it will be very tricky to do: regressions tests themselves are to be updated (new solver is almost impossible to configure to give pixel-perfect match with the old one). Since the regression tests are in separate repository you would need to make a branch first. But then, if you've got a branch in SVN, just make a branch in Git as well and use buildbot to run tests on all platforms. In fact, manta branch was tested on buildbot prior to merge, and, sure enough, the tests didn't pass. Now, if someone is not running tests locally and ignores tests failure on the branch right before the merge I don't see how CI would have helped here. > however nobody seems to be looking at the results otherwise we would have figured out the tests were failing 3 weeks ago Failure in this tests specific case I've pointed out right after the merge. Regressions tests still failing in buildbot was pointed out on Monday 23rd [1]. > and either disabled the test or fixed the problem. Please don't propose disabling legit regression tests. Those are to be fixed, not swiped under the carpet. > I kinda feel that looking at the tests on a daily basis should be on someones daily morning checklist, perhaps Nathan and/or Dalai can find someone for this task? I kind of expect it to be everyone's responsibility to: - Verify tests are passing locally prior to commit - Verify tests are passing on all platforms on buildbot. The latter one you can very easily do by glancing at a single page [2]. If the build is red something is wrong there, and is to be addressed. It could either be a compilation error or regression test failure. Both of them are to be solved and not ignored. One cruel thing i can think about is to force everyone to pay more attention is to prevent new build from being uploaded if regression tests are failing. We'll hear very quickly from users that there are no new builds on buildbot. > What is remained to do? There is a single failure now, which is related on liquid motion blur test. I've got zero clue how this is supposed to be rendered now. I can bake it and see fluid particles in viewport, but it's unclear how to make them rendered. Release Notes didn't give me any hints how to make liquid to render. [1] https://lists.blender.org/pipermail/bf-committers/2019-December/050381.html [2] https://builder.blender.org/admin/#/builders On Fri, Jan 3, 2020 at 3:50 AM Ray Molenkamp wrote: > There's a couple of things we can look at: > > We can take human element out of it, running the tests on a proposed patch > can and probably should be done automatically. I heard there were plans for > this, I hope they materialize sooner rather than later. I think this is > one of Nathans projects > > We are running the tests now on a nightly basis which is great, however > nobody seems to be looking at the results otherwise we would have figured > out the tests were failing 3 weeks ago and either disabled the test or > fixed > the problem. I kinda feel that looking at the tests on a daily basis should > be on someones daily morning checklist, perhaps Nathan and/or Dalai can > find > someone for this task? > > as for commit messages, we have good guidance there [1], having people try > a > little harder will probably go a long way there. > > --Ray > > [1] https://wiki.blender.org/wiki/Style_Guide/Commit_Messages > > > > On 2020-01-02 7:04 p.m., Sebastián Barschkis wrote: > > Hi Ray, > > > > you’re right, the Cycles tests need to be updated. They need to make use > of the new fluid modifier. > > Sergey pointed this out to me right after (within 12h) I landed the > commits, i.e. the issue is known. > > > > So yes, blame it on me, I overlooked this part. I will look into this as > soon as possible. > > > > I agree that this is not ideal. But apart from better commit messages, > how can we do better at this time? > > > > Happy new year and best wishes, > > Sebastián > > > >> On 2. Jan 2020, at 22:31, Ray Molenkamp wrote: > >> > >> All, > >> > >> Hate to be a heckler for running the unit tests, but please: > >> > >> When
Re: [Bf-committers] Mantaflow landing and unit tests.
There's a couple of things we can look at: We can take human element out of it, running the tests on a proposed patch can and probably should be done automatically. I heard there were plans for this, I hope they materialize sooner rather than later. I think this is one of Nathans projects We are running the tests now on a nightly basis which is great, however nobody seems to be looking at the results otherwise we would have figured out the tests were failing 3 weeks ago and either disabled the test or fixed the problem. I kinda feel that looking at the tests on a daily basis should be on someones daily morning checklist, perhaps Nathan and/or Dalai can find someone for this task? as for commit messages, we have good guidance there [1], having people try a little harder will probably go a long way there. --Ray [1] https://wiki.blender.org/wiki/Style_Guide/Commit_Messages On 2020-01-02 7:04 p.m., Sebastián Barschkis wrote: > Hi Ray, > > you’re right, the Cycles tests need to be updated. They need to make use of > the new fluid modifier. > Sergey pointed this out to me right after (within 12h) I landed the commits, > i.e. the issue is known. > > So yes, blame it on me, I overlooked this part. I will look into this as soon > as possible. > > I agree that this is not ideal. But apart from better commit messages, how > can we do better at this time? > > Happy new year and best wishes, > Sebastián > >> On 2. Jan 2020, at 22:31, Ray Molenkamp wrote: >> >> All, >> >> Hate to be a heckler for running the unit tests, but please: >> >> When you land and/or review something big, RUN THE UNIT TESTS! >> >> When the monster 10 patch mantaflow patch landed, it broke >> 6 cycles unit tests on all platforms, 5 with different renders >> than the reference images and one full on blender crash [1] >> >> bda4a284d20164fec2433f7c40f49fc903319400 [2] fixed the render >> differences and turned the crash into a render difference [3] >> >> Unrelated side note: whats with the less than helpful >> commit message on this commit? it may as well have been >> committed with the message 'something fluid' or 'Tuesday' >> would have just as enlightening for other developers. >> >> To summarize: >> >> - The person submitting the patches has not run the tests >> - The reviewers have not run the tests >> - Less than optimal commit messages >> - 18!! days after landing, there is still a failing test >> >> Holiday season or not: I think we can and should do better than this >> >> --Ray >> >> [1] https://i.imgur.com/LE3baOg.png >> [2] https://developer.blender.org/rBbda4a284d20164fec2433f7c40f49fc903319400 >> [3] https://i.imgur.com/5we0hEv.png >> >> >> ___ >> Bf-committers mailing list >> Bf-committers@blender.org >> https://lists.blender.org/mailman/listinfo/bf-committers > ___ > Bf-committers mailing list > Bf-committers@blender.org > https://lists.blender.org/mailman/listinfo/bf-committers ___ Bf-committers mailing list Bf-committers@blender.org https://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] Mantaflow landing and unit tests.
Hi Ray, you’re right, the Cycles tests need to be updated. They need to make use of the new fluid modifier. Sergey pointed this out to me right after (within 12h) I landed the commits, i.e. the issue is known. So yes, blame it on me, I overlooked this part. I will look into this as soon as possible. I agree that this is not ideal. But apart from better commit messages, how can we do better at this time? Happy new year and best wishes, Sebastián > On 2. Jan 2020, at 22:31, Ray Molenkamp wrote: > > All, > > Hate to be a heckler for running the unit tests, but please: > > When you land and/or review something big, RUN THE UNIT TESTS! > > When the monster 10 patch mantaflow patch landed, it broke > 6 cycles unit tests on all platforms, 5 with different renders > than the reference images and one full on blender crash [1] > > bda4a284d20164fec2433f7c40f49fc903319400 [2] fixed the render > differences and turned the crash into a render difference [3] > > Unrelated side note: whats with the less than helpful > commit message on this commit? it may as well have been > committed with the message 'something fluid' or 'Tuesday' > would have just as enlightening for other developers. > > To summarize: > > - The person submitting the patches has not run the tests > - The reviewers have not run the tests > - Less than optimal commit messages > - 18!! days after landing, there is still a failing test > > Holiday season or not: I think we can and should do better than this > > --Ray > > [1] https://i.imgur.com/LE3baOg.png > [2] https://developer.blender.org/rBbda4a284d20164fec2433f7c40f49fc903319400 > [3] https://i.imgur.com/5we0hEv.png > > > ___ > Bf-committers mailing list > Bf-committers@blender.org > https://lists.blender.org/mailman/listinfo/bf-committers ___ Bf-committers mailing list Bf-committers@blender.org https://lists.blender.org/mailman/listinfo/bf-committers
[Bf-committers] Mantaflow landing and unit tests.
All, Hate to be a heckler for running the unit tests, but please: When you land and/or review something big, RUN THE UNIT TESTS! When the monster 10 patch mantaflow patch landed, it broke 6 cycles unit tests on all platforms, 5 with different renders than the reference images and one full on blender crash [1] bda4a284d20164fec2433f7c40f49fc903319400 [2] fixed the render differences and turned the crash into a render difference [3] Unrelated side note: whats with the less than helpful commit message on this commit? it may as well have been committed with the message 'something fluid' or 'Tuesday' would have just as enlightening for other developers. To summarize: - The person submitting the patches has not run the tests - The reviewers have not run the tests - Less than optimal commit messages - 18!! days after landing, there is still a failing test Holiday season or not: I think we can and should do better than this --Ray [1] https://i.imgur.com/LE3baOg.png [2] https://developer.blender.org/rBbda4a284d20164fec2433f7c40f49fc903319400 [3] https://i.imgur.com/5we0hEv.png ___ Bf-committers mailing list Bf-committers@blender.org https://lists.blender.org/mailman/listinfo/bf-committers
