Code review comment for lp:~j-percival/fluidity/periodic-adaptivity-at-first-timestep

Revision history for this message
James Robert Percival (j-percival) wrote :

Thanks for the comments Stephan.

To address your questions:

I was slightly at a loss at what precisely to test for, so copied the basic structure of the periodic_adaptivity test, checking that the problem exited gracefully, produced output and that the maximum of the interpolated field was correct. Given my understanding of the code path, I suspect the second test is in fact redundant. However, I suspect there are ways the code could fail to produce a usable mesh while still producing output which passes the tests.

I would argue there is value in having a separate test problem for the adapt_at_first_timestep routines, since a) the code path is sufficiently individual that bugs such as the one fixed here can slip through and b) as written I think switching on adapt_at_first_timestep will cause the subsequent mesh adapts to be "do nothing" events However, given that the input mesh is identical I agree that the two tests can be combined (i.e. a single xml file which runs 2 flmls and tests both outputs). Actually is this what you meant?

« Back to merge proposal