Extend tests to cover more .fac file examples

[rs028]

I am thinking about rationalizing and extending the tests. At the moment they are a bit similar to each other and often test only specific parts of the code (sometimes very specific, for example single_reac_of_interest). I think each test may be used to test a few (unrelated, if possible) settings and/or initial conditions or outputs at the same time. Thoughts?

[spco]

Yes, I totally agree. We should attempt to reduce the overlap between tests, although I wouldn't push that too far, in case we start to lose previous test coverage.

Looking at the tests we have in place, I thinkshort_extended and single_reac_of_interest should be tweaked. The particular place we should be trying to cover is environmentVariables.config, which has lots of options, but the current tests all largely use the same choices.

As a first draft of changes to make, could I suggest: (1) short_extended to use fixed RH, with H2O = CALC. (2) single_reac_of_interest to use DEC = CALC, ROOFOPEN = 0 (3) leave most of the spec... tests as they are (don't want to change too many variables between tests) except perhaps change spec_yes_env_yes and spec_no_env_yes1 to use a more helpful constrained variable - since BLHEIGHT is never used in any calculations, constraining it doesn't have much effect on the solution!

What do you think of those suggestions? Happy to talk it through more, that's just an initial suggestion.

P.S. as I've touched on in (3) above, none of the source code itself makes use of either blh or dilute. Taking a look at the .fac files, I don't see them using it either. Is it worth keeping these around in case of a hand-crafted .fac file that uses them? Or is there an option in the MCM to use these? I guess my point is that full.fac should contain just about all the reactions, and none use either of these. Happy to leave them in there if there is a possibility of them being used - otherwise they are just clutter!

[rs028]

Sounds good. This is what I was thinking:

1) Test the single reaction of interest in short and get rid of that test 2) have a few short_extended tests to check all the environmentVariables.config options 3) the spec_* tests as they are now, with some tweaks 4) a full test with most options turned on

BLHEIGHT and DILUTE are meant to be used if you want to add processes to the base MCM model. I can add at least one of them in the full test.

What do you think?

[spco]

1. single_reac_of_interest was created to check that behaviour seen in #34 and #81. I'd agree that this is fixed and unlikely to return, so removing the test is fine.
2. Yes, perhaps variations of config options while using the same basic MCM selection is a good way to organise it.
3. Yes
4. Yes - I like the idea of adding a BLHEIGHT and DILUTE dependence to the full test. Try placing them into a fairly sensitive/significant reaction, which should highlight if there is any issue. Perhaps have BLHEIGHT fixed and DILUTE constrained, or vice versa?

[rs028]

Okay, I am happy to take this over, as it will give me a good feeling of the status and performance of the model.

BLHEIGHT is easier to add than DILUTE, because the latter is used for very specific simulations (eg, an environmental chamber).

Adding reactions and processes to the base model it's very common, and it relates to #17 (maybe that is too complicated to implement, but ideally there should be a procedure to add environment constraints, eg, instruction on which parts of the code need to be modified, if that makes sense)