Update regression tests (closes #19)

[dmey]

This PR changes the way regression test cases are run. These are now run in two different ways:

• specifying a whole case: run using configuration files from the tests/cases folder. Tests are run automatically for all case folders included in the tests/cases folder and can be easily expanded by adding a new case folders. Currently we run tests for case 103.

• patching an existing example: run using configuration files from the examples folder but with a reduced simulation time to cut CPU costs. This is done by patching namelists with patch files included in the tests/patches folder. Tests are run automatically for all patch files included in the tests/patches folder is detected and can be easily expanded by adding a new patch files corresponding to the additional example case to run. Currently we run tests for example case 001, 102, 201, 501 and 502.

[bss116]

We should leave the test case 002 in for now. The setup tests a longer run, as we previously experienced issues with simulations breaking not right at the start. This hasn't happened in a while, but for now it's good to check it nevertheless. We can however simplify the namoptions and input files, I can go over this once you put them back in.

@samoliverowens test case 002 seems to be one large facet covering the floor, with temperature, moisture etc. all on except for the EB (switched off I suppose because it does not yet work in this configuration).

[samoliverowens]

We should leave the test case 002 in for now. The setup tests a longer run, as we previously experienced issues with simulations breaking not right at the start. This hasn't happened in a while, but for now it's good to check it nevertheless. We can however simplify the namoptions and input files, I can go over this once you put them back in.

@samoliverowens test case 002 seems to be one large facet covering the floor, with temperature, moisture etc. all on except for the EB (switched off I suppose because it does not yet work in this configuration).

Yes that's right - do we want to keep it like this, or just have no blocks and facets like 001?

[samoliverowens]

This is the case I was thinking of when originally implementing lflat. It's not possible to generate this geometry using the preprocessing any more, but it would be fairly easy to put in.

[bss116]

Yes that's right - do we want to keep it like this, or just have no blocks and facets like 001?

I'd say we keep it in like this. The simpler setup is already tested in 001, and things are more likely to break in the newer parts of the code (i.e. facets, EB related things). The only thing that is not explicitly tested in there is the immersed boundary method, since there are no actual building blocks. But I think that's fine, they get tested with the other cases.

This is the case I was thinking of when originally implementing lflat. It's not possible to generate this geometry using the preprocessing any more, but it would be fairly easy to put in.

Yeah exactly, that's why I think it is good to keep this setup in the tests, in case we ever want to implement this it, we kept track that changes to the code didn't break anything there.

[dmey]

We should leave the test case 002 in for now

@bss116 sure but I would rather add it as an extra example if the old 002 is different to the current 002. Can you confirm? If so what shall we call it?

[samoliverowens]

Since (old) 002 isn't using the energy balance or scalars, shall I delete the corresponding files?

[bss116]

We should leave the test case 002 in for now

@bss116 sure but I would rather add it as an extra example if the old 002 is different to the current 002. Can you confirm? If so what shall we call it?

We have a system in place for naming examples (see https://github.com/uDALES/u-dales/issues/39#issuecomment-614588505): 0xx - neutral, 1xx non-neutral, 2xx energy balance, 5xx driver sims, 8xx trees etc.

It is a non-neutral simulation, towards using the energy balance, but not actually using it. So I'd go with either 103 or 202 ;) But don't add at as an extra example in the example folder, as it is now it's not a meaningful example.

Since (old) 002 isn't using the energy balance or scalars, shall I delete the corresponding files?

Yes, could you pick up the removed tests/cases/002 namoptions and create all the input files from scratch with the post-processing, for consistency?

[samoliverowens]

We have a system in place for naming examples (see #39 (comment)): 0xx - neutral, 1xx non-neutral, 2xx energy balance, 5xx driver sims, 8xx trees etc.

It is a non-neutral simulation, towards using the energy balance, but not actually using it. So I'd go with either 103 or 202 ;) But don't add at as an extra example in the example folder, as it is now it's not a meaningful example.

I'd say 103 if it's not using the EB.

Yes, could you pick up the removed 002 namoptions and create all the input files from scratch with the post-processing, for consistency?

Will do, so the current namoptions file (test/cases/002/namoptions.inp.002) is as we want it?

[dmey]

It is a non-neutral simulation, towards using the energy balance, but not actually using it. So I'd go with either 103 or 202 ;) But don't add at as an extra example in the example folder, as it is now it's not a meaningful example.

OK but this may unnecessarily increase the complexity of the current testing framework for just one test. At the moment we just run tests based on examples and to activate new tests one can just add the corresponding patch in the folder. Cou;ld we not make something sensible out of 103 so that we can add it to the examples?

[bss116]

Will do, so the current namoptions file (test/cases/002/namoptions.inp.002) is as we want it?

I dumped a namoptions.103 file there. Only change is the BC for moisture, which is iwallmoist = 2 (was 1 before), which should be the same as in 201, right?

It is a non-neutral simulation, towards using the energy balance, but not actually using it. So I'd go with either 103 or 202 ;) But don't add at as an extra example in the example folder, as it is now it's not a meaningful example.

OK but this may unnecessarily increase the complexity of the current testing framework for just one test. At the moment we just run tests based on examples and to activate new tests one can just add the corresponding patch in the folder. Cou;ld we not make something sensible out of 103 so that we can add it to the examples?

Well this is a 8 x 8 x 8 m blob with all our model capabilities on, not sure what this is supposed to represent 😄 we could do a tests folder within examples...?

[bss116]

@samoliverowens sorry forgot the poisson solver there. @dmey why does it still run the checks even if I put [skip CI] in the commit message?

[dmey]

@samoliverowens I have moved 103 under tests/cases, please can you add the files in there. I will then add that to the tests.

@dmey why does it still run the checks even if I put [skip CI] in the commit message?

@bss116 thanks for noticing this. I'll fix this in a separate PR.

[samoliverowens]

I've created a separate PR (#113) in order to enable the preprocessing to generate the geometry for 103, and added the input files created from your namoptions file @bss116 - I've just added the necessary switch (lfloors) in &INPS.

[bss116]

I've created a separate PR (#113) in order to enable the preprocessing to generate the geometry for 103, and added the input files created from your namoptions file @bss116 - I've just added the necessary switch (lfloors) in &INPS.

Shall we merge this into master or into this branch directly?

[dmey]

Shall we merge this into master or into this branch directly?

I'd do master here

[dmey]

@samoliverowens and @bss116 when you have a moment can you please merge #113 in so that I finish with the extra tests here

[samoliverowens]

@samoliverowens and @bss116 when you have a moment can you please merge #113 in so that I finish with the extra tests here

All done!

[dmey]

@bss116 @samoliverowens tests for 103 are now in and are run separately -- I have included updated instructions on how to add new cases etc in the readme.

Case 201, 501, 502 still take too long in my opinion, can we cut the runtime in half?

[bss116]

@bss116 @samoliverowens tests for 103 are now in and are run separately -- I have included updated instructions on how to add new cases etc in the readme.

Case 201, 501, 502 still take too long in my opinion, can we cut the runtime in half?

Yes, you can further reduce the runtime. For 501+502, change also to driverstore = 11.

I'm not quite understanding the developer notes. If I want to run the code checks (not the docs) and I have all the python packages already there, do I just need to run the run_tests.py script?

[dmey]

@bss116 @samoliverowens tests for 103 are now in and are run separately -- I have included updated instructions on how to add new cases etc in the readme. Case 201, 501, 502 still take too long in my opinion, can we cut the runtime in half?

Yes, you can further reduce the runtime. For 501+502, change also to driverstore = 11.

Cool, I reduced the runtime now and set driverstore = 11.

I'm not quite understanding the developer notes. If I want to run the code checks (not the docs) and I have all the python packages already there, do I just need to run the run_tests.py script?

Yes -- the only reason I assume conda is because we test it and know it will work out of the box.

[bss116]

@bss116 @samoliverowens tests for 103 are now in and are run separately -- I have included updated instructions on how to add new cases etc in the readme. Case 201, 501, 502 still take too long in my opinion, can we cut the runtime in half?

Yes, you can further reduce the runtime. For 501+502, change also to driverstore = 11.

Cool, I reduced the runtime now and set driverstore = 11.

How many time steps do we want the code to run? one second may mean we only have one or two in 201. I suppose we want two at the very least? If you want to tune it precisely, you can set a dtmax under &RUN, as it is done in 501 and 502, where we now test 10 timesteps (runtime = (driverstore-1)*dtdriver)

I'm not quite understanding the developer notes. If I want to run the code checks (not the docs) and I have all the python packages already there, do I just need to run the run_tests.py script?

Yes -- the only reason I assume conda is because we test it and know it will work out of the box.

Okay I see. And for what is Graphviz needed, that is mentioned in the developer notes?

[dmey]

@bss116 @samoliverowens tests for 103 are now in and are run separately -- I have included updated instructions on how to add new cases etc in the readme. Case 201, 501, 502 still take too long in my opinion, can we cut the runtime in half?

Yes, you can further reduce the runtime. For 501+502, change also to driverstore = 11.

Cool, I reduced the runtime now and set driverstore = 11.

How many time steps do we want the code to run? one second may mean we only have one or two in 201. I suppose we want two at the very least? If you want to tune it precisely, you can set a dtmax under &RUN, as it is done in 501 and 502, where we now test 10 timesteps (runtime = (driverstore-1)*dtdriver)

But isn't increasing tstep the same as increasing the sim time with regards to CPU time? If that's the case then I might as well leave the old sim time.

I'm not quite understanding the developer notes. If I want to run the code checks (not the docs) and I have all the python packages already there, do I just need to run the run_tests.py script?

Yes -- the only reason I assume conda is because we test it and know it will work out of the box.

Okay I see. And for what is Graphviz needed, that is mentioned in the developer notes?

To create the graphs used in the software docs (which I noticed they are actually not being rendered -- so will fix that separatly). Btw it's an optional requirement so if you don;t have it installed it just doesn't create graphs

[bss116]

@bss116 @samoliverowens tests for 103 are now in and are run separately -- I have included updated instructions on how to add new cases etc in the readme. Case 201, 501, 502 still take too long in my opinion, can we cut the runtime in half?

Yes, you can further reduce the runtime. For 501+502, change also to driverstore = 11.

Cool, I reduced the runtime now and set driverstore = 11.

How many time steps do we want the code to run? one second may mean we only have one or two in 201. I suppose we want two at the very least? If you want to tune it precisely, you can set a dtmax under &RUN, as it is done in 501 and 502, where we now test 10 timesteps (runtime = (driverstore-1)*dtdriver)

But isn't increasing tstep the same as increasing the sim time with regards to CPU time? If that's the case then I might as well leave the old sim time.

It's adaptive time stepping, so we can only set a maximum. I meant that you could set a very small maximum, say 0.2, and then set the runtime to 0.6 to get exactly three time steps. But it's also good to check that the adaptive time step does work, so I'd say if you just use 2.5 runtime in 201 then we should get enough time steps to check (time steps are usually 0.5 or smaller). Would that work or is it still too slow?

Okay I see. And for what is Graphviz needed, that is mentioned in the developer notes?

To create the graphs used in the software docs (which I noticed they are actually not being rendered -- so will fix that separatly). Btw it's an optional requirement so if you don;t have it installed it just doesn't create graphs

Okay cool. Would be good to mention that it's not needed, but we can do that separately (e.g. in #117).

[dmey]

@bss116 @samoliverowens tests for 103 are now in and are run separately -- I have included updated instructions on how to add new cases etc in the readme. Case 201, 501, 502 still take too long in my opinion, can we cut the runtime in half?

Yes, you can further reduce the runtime. For 501+502, change also to driverstore = 11.

Cool, I reduced the runtime now and set driverstore = 11.

How many time steps do we want the code to run? one second may mean we only have one or two in 201. I suppose we want two at the very least? If you want to tune it precisely, you can set a dtmax under &RUN, as it is done in 501 and 502, where we now test 10 timesteps (runtime = (driverstore-1)*dtdriver)

But isn't increasing tstep the same as increasing the sim time with regards to CPU time? If that's the case then I might as well leave the old sim time.

It's adaptive time stepping, so we can only set a maximum. I meant that you could set a very small maximum, say 0.2, and then set the runtime to 0.6 to get exactly three time steps. But it's also good to check that the adaptive time step does work, so I'd say if you just use 2.5 runtime in 201 then we should get enough time steps to check (time steps are usually 0.5 or smaller). Would that work or is it still too slow?

cool reverted to 2.5 s in 8894987 we can always change it in the future if we think this takes too long

Okay I see. And for what is Graphviz needed, that is mentioned in the developer notes?

To create the graphs used in the software docs (which I noticed they are actually not being rendered -- so will fix that separatly). Btw it's an optional requirement so if you don;t have it installed it just doesn't create graphs

Okay cool. Would be good to mention that it's not needed, but we can do that separately (e.g. in #117). 👍

[bss116]

Let's also change the stats to 2 seconds (you always need something slightly lower than the runtime, if it's exactly the runtime then the stats don't complete) and then it's done

[dmey]

Let's also change the stats to 2 seconds (you always need something slightly lower than the runtime, if it's exactly the runtime then the stats don't complete) and then it's done

What do you mean? They are set to 1 s at the moment.

[bss116]

Let's also change the stats to 2 seconds (you always need something slightly lower than the runtime, if it's exactly the runtime then the stats don't complete) and then it's done

What do you mean? They are set to 1 s at the moment.

Let's set them to two seconds in 201