Shared library

[spco]

Addresses #369.

Convert the .fac file to a standalone Fortran module, which is dynamically linked as a .so to the main AtChem2 executable.

[codecov]

Codecov Report

Merging #370 into master will decrease coverage by 0.13%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master     #370      +/-   ##
==========================================
- Coverage   90.42%   90.28%   -0.14%     
==========================================
  Files          15       15              
  Lines        2109     2121      +12     
==========================================
+ Hits         1907     1915       +8     
- Misses        202      206       +4

Flag	Coverage Δ
#build	65.32% <66.66%> (ø)	⬆️
#tests	89.62% <66.66%> (-0.14%)	⬇️
#unittests	34.7% <0%> (-0.2%)	⬇️

Impacted Files	Coverage Δ
src/solverFunctions.f90	87.91% <66.66%> (-3.23%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f298ef0...3cdb8d3. Read the comment docs.

[spco]

@rs028 I think this is ready for review. The only failures are from the code coverage, which I don't think I can do much about.

Work remaining: Update the README (and the wiki, once v1.1 docs are out) with the changes to how things run.

In particular, I'd value you thoughts on whether the new shared library should normally go into model/configuration (or travis/tests/testname/model/configuration for tests). Is this a sensible place? It matches where the .fac lives for the tests, but the build test uses tools/mcm_example.fac, so the .so ends up apart from the .fac (but with the other config files, including the other mechanism.* files).

[rs028]

Good question. I tend to put the .fac file in model/ rather than model/configuration so that the latter contains only .config and .parameters files. I have no problem with the shared library residing in model/ or in model/configuration/. Or there could be an additional directory model/mechanism.

With regard to mcm_example.fac I am wondering if tools is the right place for it. Maybe it should go into mcm/ and be renamed mechanism_test.fac. That being said, is it possibile to force the shared library to be wherever the mechanism file happens to be? It would be the easiest solution.

[spco]

This feeds into a wider discussion of how flexible we want to be, versus how easy we want it to be to not go wrong. Currently tools/setup_atchem.py take up to 4 arguments, and similarly tools/build.sh takes 4 arguments which it passes on. The atchem2 executable then has 7 positional command line arguments (5 for input directories, and 2 for output directories). I'm wondering whether that is potentially too many options, and we would be wise to cut these down a little, in order to enforce some sanity by users (for example, I don't really want to live in a state where users can specify unrelated locations for spec_constraints_dir, env_constraints_dir, and photolysis_constraints_dir - they should all be subdirectories of the same directory).

I'd be interested in your thoughts on this. Of course, if we get round to moving to a smarter command line interface, where we accept named arguments rather than just positional ones, then that might make it easier to offer a simple hard-to-break default interface, with more powerful options available if users really want to use them - but make it far easier to just plump for an easy setup.

As an example, I'm thinking atchem2 really just wants to be told the locations of (a) the shared library for the mechanism; (b) the MCM photolysis files; (c) the input configuration for a single model (encompassing the subdirectories for constraints etc); and (at a stretch, possibly) (d) an output directory.

In extremis, users also have access to more granular arguments specifying e.g. environment constraints come from some other location.

In essence, I want it to be easy to just point at the root directories for whole models, and a bit harder to specify more granular details - that way, you nudge users to do sane things like storing all the data for one model in the same location, and storing the output of the model in the same location.

[spco]

Back to the matter in hand - yes, we can force the location of the shared library to that of the mechanism file, and I think that's sane behaviour (in fact the only reason this doesn't do that currently is because that wouldn't work for mcm_example.fac's current location).

I can see an argument for the default being model/, as you're right that it's not really config in a natural sense. Perhaps model/mechanism might even be preferable? That way, the .fac, mechanism.f90, mechanism.o and mechanism.so are all bundled neatly together.

[rs028]

This all sounds very sensible. Having a lot of arguments is a bit of a pain, but also can cause to errors. With or without a smarter command line interface, it may be a good idea to simplify and nudge towards good behaviour.

Possibily the easiest solution is to decide that the directory $MODEL must have 4 sub-directories with specific names: configuration/, constraints/, output/ and maybe mechanism/. In this way the user only needs to specify 1 argument. But maybe that would be too rigid.

This is probably a different PR? Or needs to be resolved now to finalize this one?

[spco]

I'd suggest if you're happy for now, we merge this PR and open a new one. I might even have a look at improving the interface. Are you happy if this PR goes ahead?

[rs028]

Yes, okay.

[spco]

Hi @rs028 - I'm back from leave so looking to wrap this up. Are you happy if I rebase this and merge into 1.2-dev (which may want to be 2.0-dev in the end)? I can also open the other PR for the restructuring as discussed above.

[rs028]

Welcome back :) Yes, I am happy to do so.

[spco]

@rs028 Before I commit this, I want to update (a) the README.md to reflect the change in the build process. I will also want to update the documentation. Should I update the contents of doc/ in this PR, and/or the wiki pages afterwards? Or are we using the wiki as the canonical live documentation, and doc/ only contains the snapshot of the wiki at the latest release?

[rs028]

Good question. My plan was to convert the files in doc to latex and then use those as live documentation. A pdf file can be attached to each new release.

This would make the wiki redundant, as it would be a bit cumbersome to keep it in sync with the latex files, but I am not sure what the best approach is.

Right now I have changed the md files only when I closed PR #375.

[spco]

Ok, yes. I guess we should deprecate the wiki (either remove it, or add bold headers explaining that it's deprecated) in order to not confuse users.

https://github.com/palantir/godel/wiki/GitHub-wiki seems to be a useful tool (it updates the wiki to reflect the contents of doc), but remembering to run it frequently is probably a pain.

Perhaps the wiki could also be updated only at the time of release, like the LaTeX? Would that be useful to users?

[rs028]

I am leaning towards removing it once the transition to latex is complete. Maybe keep one or two wiki pages with practical examples of usage that won't fit in the manual. What do you think?

[spco]

Perhaps that's a sensible compromise - let's go with that. Point users to the main docs, but also supply some useful extras that may not be ideally suited within the main docs.