Add driver, scalar and statistics capabilities. Remove trees and purifs.

[tomgrylls]

Patching the branch of code used by tg3315 approx. November 2018 - September 2019 into the master branch. Main changes:

• Capability to run driver simulations and capability to drive simulations via a precursor simulation. Initial work done by Anton Esmail-Yakas, August 2017. Reintegrated into the code by myself in June 2019. The module moddriver.f90 uses modinlet.f90 as a reference point. The integration of this into the code therefore follows a similar structure and uses a similar set of switches that should be reviewed in Restructure and retest modinlet and moddriver. Improved modboundary. #68. As part of Restructure and retest modinlet and moddriver. Improved modboundary. #68 (and as a result of this merge) there are several potential improvements to the structure of modboundary.f90 to be implemented. This merge also increases need for Check the use of ubulk and uouttot #54 to be resolved (these variables are defined correctly for driver simulations here).
• Use of lEB to limit the allocation of the very large arrays associated with the 3DRT.
• Inlet-outlet lateral boundary conditions for scalars.
• IBM implemented for scalar fields. New input iwallscal etc.
• No longer setting initial scalar concentrations within buildings to zero. Therefore avoiding errors at startup if initial scalar profiles are non-zero.
• Changed the allocation of the largest arrays in modstatsdump.f90 to avoid exceeding HPC limits.
• More comprehensive outputs from statistics routine, including for scalar fields. This provides a more complete module from which to do Automate modstatsdump #42 and Improve statistics output modstatsdump.f90 #57 but also leads to large output files on current branch.
• Scalar sources. Ability to have more than one point source and in multiple locations. Ability to deal with situation with all blocks in lowest layer. No normalisation and specific line source set up. This module is still fairly specific to my project work but is a later iteration and will be simplified/ improved via new issue Improved/ simplified scalar source terms #69.
• Trees removed.
• Purifs removed.

[dmey]

I do not have much to contribute to this review -- others may have more to say. This seems OK to me with build and current tests not failing. The only minor comments I have are to: (a) add documentation about how to run with driver simulation on (b) add driver simulation to current tests.

[tomgrylls]

Okay I will work on some documentation and on running some sanity checks myself today.

Are we sure that we want to add this to the Travis CI tests? It would require two additional simulations - one precursor simulation to create the driver input files and a second simulation that uses these as an input. In fact I may need some help to automate that. I guess we could do this separately by checking the outputted driver files of the first simulation and then always using the same input files for the second simulation.

[tomgrylls]

@dmey Could I get some quick advice on where and how to best add documentation? E.g. on the 'Getting started guide' or elsewhere? Use of .md files?

[dmey]

Are we sure that we want to add this to the Travis CI tests? It would require two additional simulations - one precursor simulation to create the driver input files and a second simulation that uses these as an input. In fact I may need some help to automate that. I guess we could do this separately by checking the outputted driver files of the first simulation and then always using the same input files for the second simulation.

Can the run time be made really short -- a few seconds -- so not to add a substantial amount of time to the CI? And if so, can you lay out the procedure to do this somewhere in the docs (see also below)-- at the moment I have no idea about how to do this and if this will actually make sense doing. If this is straightforward we should probably add to make sure that future changes will not break this feature.

@dmey Could I get some quick advice on where and how to best add documentation? E.g. on the 'Getting started guide' or elsewhere? Use of .md files?

At the moment there are no guidelines for this -- I think we meant to discuss it but never got round to it. My personal preference would be to have all these 'advanced' features grouped in a separate document. I think that the getting started should be kept as short as possible to give users a way to start using uDALES and keep anything else in tutorials or separate documentation. @bss116 @ivosuter @samoliverowens do you have anything to add here? I see that @samoliverowens and @bss116 have already done a pretty neat job with the name list docs (see https://github.com/uDALES/u-dales/tree/so-tg/preproc/docs).

[samoliverowens]

At the moment there are no guidelines for this -- I think we meant to discuss it but never got round to it. My personal preference would be to have all these 'advanced' features grouped in a separate document. I think that the getting started should be kept as short as possible to give users a way to start using uDALES and keep anything else in tutorials or separate documentation.

I think that's the best approach - effectively splitting it into a user guide and a developer's guide, with the user guide being the priority.

[bss116]

I suggest adding/updating all parameter descriptions to the namelist docs. Since the last commit they are already in the master branch, therefore also in your branch: https://github.com/uDALES/u-dales/blob/tomgrylls/rmtrees-driver-patch/docs/udales-namoptions-overview.md.
@samoliverowens started a document on how to set up the BCs in new simulations. I think this is suitable to be expanded to a non-entry level guide with more detailed instructions. It's in the example-simulations branch, you can just copy it over to your branch: https://github.com/uDALES/u-dales/blob/bss/example-simulations/docs/udales-simulation-setup.md.

[bss116]

Just to be a little more precise on this: I suggest short parameter descriptions in the namoptions-overview and a guide on how to use them in the simulation-setup.

[tomgrylls]

I have completed documentation for the driver simulations (both updating the namoptions descriptions and adding to the simulation set-ups doc).

@dmey I have been running short test simulations on my machine so I have these available. I may need some advise as to how to set these up as tests? What file types does the CI actually do the comparison on? Options are:

1. Set up both the precursor and driven simulation to be run in series. This requires a step where the driver output files of the precursor simulation are copied across to be read in by the second driven simulation.
2. Conduct two separate tests. i) one checks the output of the precursor simulation. Ideally the binary driver files that it outputs. ii) one that runs a driven simulation using the same driver files every test and does the same test as we currently do for the periodic simulation.

[dmey]

@dmey I have been running short test simulations on my machine so I have these available. I may need some advise as to how to set these up as tests? What file types does the CI actually do the comparison on?

Sure no problems -- I will look at this tonight and will let you know if that's OK but let me know if it's urgent.

[tomgrylls]

@dmey the only urgency is that there is a masters student that needs to use scalars in some simulations ASAP and the other branches do not have any immersed boundary method for scalar fields which is a big issue for him. I can get him to start working on this branch already but Maarten prefers that he waits until this pull request is complete and then can update the pre-processing branch that he is currently using. For that reason, it would be good to get this all done in the next couple of days.

I am going to set up some test simulations with scalars today too and run a sanity check against my old branch. This could also be used as an additional test in the CI - it may be easiest to just add a scalar point source in the original test case?

[dmey]

That's fine, I will look at this tonight -- in the meantime, could add the case folder/s under https://github.com/uDALES/u-dales/tree/tomgrylls/rmtrees-driver-patch/tests/cases I will see what else needs to be added to the current test framework to support this specific case. If I understand correctly, you first need to create the outputs from a case folder (the driver), then you use those outputs to run another case (the simulation). For now if you just add two folders, under tests like case 100 and 101 for the driver and the simulation respectively, then I can see how to link these two in the actual tests.

BTW, did you look at the README on how to run tests locally (https://github.com/uDALES/u-dales/tree/tomgrylls/rmtrees-driver-patch/tests)? Ideally, tests should be run locally before pushing to CI.

[dmey]

@tomgrylls At the moment the tests we do are simply meant to check that 1. uDALES compiles 2. the cases under the tests folder run and 3. we compare the output from one branch/version to the other. We do not in any way check if the results are physically correct as this would need you to compare against a gold-standard model or a gold-standard data.

With regards to your point 1, yes feel free to add as many cases as you want -- the current framework allows you to simply add the files in a new case -- you won't need to change anything in the code. BTW we should probably add some info in the test README to say what each case test.

With regards to point 2, I think it may be just easier to have everything in one folder I have changed this and submitted a PR for this -- see #71.

[dmey]

@dmey anyway to remove some commits from the queue on these CI tests and jump to the latest push?

@tomgrylls sure just go to Travis by clicking on the CI status next to the commit ID or go to https://travis-ci.com/github/uDALES/u-dales and go to the commits you want to cancel jobs for. Then click on the X to cancel the job -- you must be login in to do this -- just click on sign in with GitHub

[dmey]

Also you can skip CI in the commit just by adding [skip ci] to you commit message -- see https://docs.travis-ci.com/user/customizing-the-build/#skipping-a-build

[tomgrylls]

Not sure why this is failing now - will have to look into it again tomorrow.

[dmey]

Not sure why this is failing now - will have to look into it again tomorrow.

I would expect this to fail if what you have added is not supported/available in the master branch. I will need to improve the logging so it's clearer which ones fails -- at the moment it's just a dump of stdout...

[tomgrylls]

I am running some final sanity checks - comparing this branch against my old branch tomgrylls/trees-driver and then will squash and merge.

[bss116]

I'm also just quickly going through the changes in the code. Have you run a check without scalars (nsv=0)? If that works, then I think we can close #42 with the merge as well, right?

[tomgrylls]

I'm also just quickly going through the changes in the code.

Great thanks.

Have you run a check without scalars (nsv=0)?

Yes I have run tests with nsv = 0 and it works fine. The new tests 101 and 102 both have nsv =0.

If that works, then I think we can close #42 with the merge as well, right?

It will correctly not do the calculations depending on the switches as mentioned #42 but the netcdf files themselves will still have all these outputs but full of zeros if uncalculated. So I do not think we can close #42 from this. This modstatsdump file is more complete though now in this regard and would make further automation (e.g. #57 (comment)) easier in a future PR.

[bss116]

If that works, then I think we can close #42 with the merge as well, right?

It will correctly not do the calculations depending on the switches as mentioned #42 but the netcdf files themselves will still have all these outputs but full of zeros if uncalculated. So I do not think we can close #42 from this. This modstatsdump file is more complete though now in this regard and would make further automation (e.g. #57 (comment)) easier in a future PR.

I agree, it would be good to output only the necessary fields. However, we thought this is a bit more work, and therefore decided to split it up in two issues: one which is basically a hack so that everything we can run can also be processed in the outputs (#42), and one where we do a proper re-write of the module (#57). As long as it does not create any overhead to the simulation (I think just writing empty fields to the netcdf shouldn't be too bad), the hacky bit is done, right?

Edit: Okay I see tdump has a lot more output, and these 3D fields are quite big...

[tomgrylls]

Edit: Okay I see tdump has a lot more output, and these 3D fields are quite big...

Yes I think splitting at least tdump up into separate files should happen quite soon. It should not be difficult doing it like that though (#57 (comment)).

[bss116]

Ok I left a couple of comments, but seems alright as far as I can tell. The use of uouttot in modboundary and elsewhere needs to be addressed soon, as well as improvements to modstatsdump, but I would suggest to treat them as different issues. Should we close #42 and remove the files modstatsdump.f90_EXTENSIONS and modstatsdump.f90_SCALARS? We can either split up tdump as part of #57 in the next milestone, or create a separate issue for it to be solved with 0.0.1.

[tomgrylls]

Okay I removed the old interchangeable modstatsdumps and resolved @bss116's comments.

I have also run 'sanity' checks between this branch and tomgrylls/trees-driver for the driver capabilities and scalar IBM, advection, diffusion and emission. I think it is now ready to squash and merge.