Order-of-magnitude slowdown in FIAT

[chrisrichardson]

Original report by Andrew McRae (Bitbucket: amcrae, GitHub: amcrae).

---

Hello all,

We at Firedrake recently incorporated the changes between 5500635 and ec68699 into our fork of FIAT (i.e. all of Nico/Aslak's recent changes)

After running our test-suite through cProfile, the time spent in FIAT's polynomial_set init or 'below' jumped from 1.28% to 18.21%, accompanied by a 15-20% increase in the total test time. Nearly all the time is spent in calls to numpy_lambdify and calls to form_derivative, both in expansions.py.

Any idea what's happened?

[chrisrichardson]

Original comment by Jan Blechta (Bitbucket: blechta, GitHub: blechta).

---

FIAT has virtually no tests.

I've already started unit testing framowork (continuing the work of Aslak and Nico). It's in master. After a long time we are also running regression test, see #3. So this has been improved recently.

Expand testing to reach decent level of coverage.

I wasn't tough enough in email discussion with Firedrake people to require them to write tests for their additions. I'll try to cover FFC elements being moved to FIAT in future.

[chrisrichardson]

Original comment by Jan Blechta (Bitbucket: blechta, GitHub: blechta).

---

FIAT now has a unit test framework, thanks to Aslak, Nico, David, Miklós, Garth, etc.

Second task is now to write unit tests. Let's not accept new features without tests.

[chrisrichardson]

Original comment by Garth N. Wells (Bitbucket: garth-wells, GitHub: garth-wells).

---

@blechta I think we could do with a new 'contributing' guide to help contributors with the process, e.g. adding tests. Something could be added to https://bitbucket.org/fenics-project/docs.

[chrisrichardson]

Original comment by Jan Blechta (Bitbucket: blechta, GitHub: blechta).

---

I'm not sure that it needs a guide. Do you think that anybody, who is able to understand a relevant piece of code and contribute there, will not be able to go through the test dir and add something relevant there when asked to do that by an integrator? I think that most important is discipline of developers/integrators not to merge anything without appropriate coverage.

[chrisrichardson]

Original comment by Lawrence Mitchell (Bitbucket: wence, GitHub: wence).

---

Yes, but it's easier for everyone if the requirement to add tests is spelt out for new contributors. And also it reminds new developers/integrators that they should pay attention to features arriving without tests.

[chrisrichardson]

Original comment by Jan Blechta (Bitbucket: blechta, GitHub: blechta).

---

Yes, that's already stated here https://fenicsproject.org/contributing/#testing. If we would like to move that to RTFD page I'd suggest to file a more general issue stating what should be done in these regards.

[chrisrichardson]

Original comment by Garth N. Wells (Bitbucket: garth-wells, GitHub: garth-wells).

---

I think it could be stated much more clearly and with more detail on the work flow, making pull requests, test framework, etc.