bcgov / wqg_data

Refining the WQG list
GNU General Public License v3.0
3 stars 3 forks source link

Condition logic #49

Closed sebdalgarno closed 4 years ago

sebdalgarno commented 4 years ago

There are many Conditions that look like this: EMS_0107 | EMS_1107 > 10

I assume in R expression this should look like: EMS_0107 > 10 | EMS_1107 > 10

If we look at 'Fluoride Total' - 'Freshwater Life - Aquatic', there are two possible guidelines depending on Conditions:

EMS_0107 > 10 | EMS_1107 > 10
EMS_0107 <= 10 | EMS_1107 <= 10

if EMS_0107 is 9 and EMS_1107 is 12, then both conditions evaluate to being TRUE and two guidelines will be shown.

is this the desired behaviour?

sebdalgarno commented 4 years ago

flawed Condition logic for Alkalinity (Total CaCO3): "EMS_CA_D >= 4 | EMS_CA_D <= 8" (and other similar expressions) This will always evaluate to TRUE no matter the value of EMS_CA_D

joethorley commented 4 years ago

This "EMS_CA_D >= 4 | EMS_CA_D <= 8" should presumably be replaced with

"EMS_CA_D >= 4 & EMS_CA_D <= 8"

sebdalgarno commented 4 years ago

yes that's what I thought too - just wanted confirmation

atillmanns commented 4 years ago

Hi @sebdalgarno , for the example at the top:

if EMS_0107 is 9 and EMS_1107 is 12, then both conditions evaluate to being TRUE and two guidelines will be shown.

The user should only ever be able to give one value of hardness to calculate the guidelines so if the user tries to add both dissolved and total hardness, then the app should default to use dissolved hardness.

sebdalgarno commented 4 years ago

OK great @atillmanns ... I will ensure that dissolved hardness has priority. Do you agree with @joethorley changes to the condition logic above? (EMS_CA_D >= 4 | EMS_CA_D <= 8)

atillmanns commented 4 years ago

I think so... in words it should read that the pH must be greater than or equal to 4 and less than or equal to 8; (basically between a pH of 4 and 8).

sebdalgarno commented 4 years ago

Ok perfect thanks... will make those changes

sebdalgarno commented 4 years ago

hi @atillmanns ... I'm still a bit confused about the Hardness Total/Hardness Dissolved logic. The Limit equation only has Hardness Total (EMS_0107)...so if Hardness Dissolved (EMS_1107) is provided and not Hardness Total, the guideline cannot be calculated.

Basically, it seems that Hardness Total is required to calculate the guideline so I'm not sure I see the point of giving the user option to provide Hardness Dissolved.

Again, this occurs with 'Fluoride Total' - 'Freshwater Life - Aquatic'

atillmanns commented 4 years ago

hey @sebdalgarno , yes it is a bit confusing. Hardness can modify the toxicity of specific substances so guidelines for these substances are generally given as hardness based equations. Typically, in the field, people measure total hardness, but the experimental data that was used to generate the equation was based on dissolved hardness. So although dissolved hardness is technically the correct form to use, people often don't have the dissolved hardness value and so they can use total hardness to estimate the guideline. So I went through and changed all the logical conditions to accept both forms of hardness, but I see your point. I didn't change the equations to accept both forms of hardness.... I suppose I could go in and do this though I'm not sure exactly how to write the expressions...

atillmanns commented 4 years ago

Perhaps I should create a new column for the equations for dissolved hardness?

sebdalgarno commented 4 years ago

I think we can just leave the equations the way they are. In the app, if the user provides Hardness Dissolved, we will pass it to the equation as if it is Hardness Total (since the guideline will be the same either way)

atillmanns commented 4 years ago

okay, thanks Seb!!