Improvements to environmentVariables.config

[rs028]

• Make sure that H2O and RH are mutually exclusive (if one is constrained or constant the other must be calculated).

• Make sure that the text in environmentVariables.config is unequivocal and intuitive (eg ROOFOPEN=ON is a confusing, ROOF=OFF is better).

• Simplify options in environmentVariables.config where possibile (eg, JFAC=1 and JFAC=NOTUSED are the same).

• What happens if there is a typo (eg, "CONTRAINED" instead of "CONSTRAINED") or an option that is not allowed (eg, "NOTUSED" for TEMP) in environmentVariables.config?

• If DILUTE is in the chemical mechanism, but it is set to NOTUSED in environmentVariables.config (by mistake), the results are affected. This should not happen: the model should raise an error because it is not clear what the user wants to do. This may also happen for other environment variables (need to check!)

• Check numbers in the first column: H2O and DEC have the same number, does it matter?

[spco]

Looking at (5) about DILUTE, this is a subtle issue.

If DILUTE, BLHEIGHT or RH are set to NOTUSED, they end up having envVarTypesNum = 4. This is then handled on l316 of constraintFunctions.f90 in getEnvVarsAtT(). Here, they are set to a 'nonsense value' of -1, but no error is thrown.

Ideally, we'd have an error triggered here if DILUTE is set to NOTUSED, but we're trying to evaluate it.

The difficulty comes that in the current implementation, we always evaluate all of the environment variables at each time of calling getEnvVarsAtT() - this function has no knowledge of whether our mechanism will actually make use of these values, so it just sets some nonsense. It's up to the user to ensure they never actually use that nonsense value - the idea of using -1 is that it should hopefully give weird enough results that the user takes a deeper look and spots the error.

I'm not quite sure how to easily handle this for now - we'd need to do something where either (1) the mechanism is checked to ensure it matches the environmentVariables.config and doesn't use a NOTUSED variable; or (2) somehow inform getEnvVarsAtT() about the contents of the mechanism. Neither is simple.

This also kind of raises the question of why we'd ever want to put NOTUSED for DILUTE in the environmentVariables.config - if our mechanism file demands we use DILUTE, is the NOTUSED just to act as a warning to the user?

[rs028]

So i think the weirdness in the data may be too subtle depending on how DILUTE is used but I also think we don't need to try to pre-check here because it is clearly the user who made a mistake and we can't second guess what they wanted to do.

How about instead of a number we use a character, such as NA? So it the model tries a mathematical operation on it, it will crash.

Same behaviour for BLHEIGHT, while for RH the problem should be solved by 1) in this issue.

I agree with the last point you make, though. The behaviour should be changed but I would put in its own issue.

[spco]

Unfortunately Fortran uses strong typing, so we can't optionally set DILUTE to anything other than a real.

We can use NaNs instead (which would tell the user that something's gone wrong when they look at the output, but wouldn't crash the program explicitly), but that brings its own issues - this isn't supported in gfortran 4.8, (only in 5+). 4.8 is ancient, but it tends to be the default system version on some systems (e.g. Alice2, but there we can load modules of newer gcc versions).

Or we can come up with another 'default' value that is so absurd that it will always show up to the user. Thoughts?

[rs028]

What about a type conversion? The input is a always a string and converted to a number before usage. If the conversion fails it throws an error and alerts the user. Maybe not an elegant solution, but would that work?

[spco]

Hmm. You'd need to replace every usage of DILUTE with something like a function get_dilute(), which would do the conversion on the fly. I think that would cripple performance, as that conversion would be done every single time DILUTE is used - potentially rather a lot. And we'd really only want to perform such a check the first time DILUTE is to be used. It would also require something of a rewrite of how we handle environment variables - we currently update only once per timestep, rather than recalculating at each call of each variable. Seems like we'd lose out a lot just to catch a corner case. I think there must be a better way.

[rs028]

I was thinking this: https://gcc.gnu.org/onlinedocs/gfortran/REAL.html
or is it also a fortran 5 function?

[spco]

REAL just converts from int or complex, not from string.

[rs028]

Ah, okay. How about a python script to parse the whole environmentVariables.config file for consistency before parsing the model. Maybe it could help with the other points in this issue. Just an idea.

[spco]

Yes, that's probably more feasible, but adds another possible layer. I'll have a think.

[rs028]

I have received some feedback on this. It would probably make more sense to redefine DILUTE so that it is automatically included in all reactions. This can be treated together with #17 in the sense that the current DILUTE is renamed and used as a different environment variable and a new DILUTE switch is added to the code.

[spco]

I'm not entirely sure that I understand fully. Are you saying that the DILUTE line in environmentVariables.config should just be ON/OFF? What do you mean by 'automatically included in all reactions'?

[rs028]

No, it is a numeric value set in environmentVariables.config. If there is dilution then it should be active for all species. Therefore it makes more sense to tell the model to apply the given dilution rate to all species, rather than requesting the user to modify the chemical mechanism to account for that. Unless I am mistaken as to how it is currently implemented.

[spco]

I see. So dilution is a constant valued rate of species concentration decrease? As in, at each timestep we lose some fixed percentage of every species' concentrations?

[rs028]

Essentially yes.

[rs028]

Point n.5 is resolved by PR #408.