search

osparcomm / HARSAT

Harmonized Regional Seas Assessment Tool
https://osparcomm.github.io/HARSAT/
Other
0 stars 5 forks source link

merge determinand.link.sum and determinand.link.weights #470

Closed RobFryer closed 3 months ago

RobFryer commented 5 months ago

The functions determinand.link.sum and determinand.link.weights are very similar. One computes the sum of concentrations to get a total concentration, and the other computes the weighted sum of concentrations when e.g. there are TEFs to get a total TEQ.
It would be sensible to streamline the code by merging them. This would involve adding a weights argument to determinand.link.sum and taking the best bits of each function.

annelaerkes commented 5 months ago

@RobFryer Rob: please can we put spaces between the arguments to function calls - it makes the code much more readable (for me, at least!) Anne: I took care to try to follow the style used in the code, but obviously still got it wrong. But it is not clear to me what I am doing wrong? Can you give an example? Thanks

RobFryer commented 5 months ago

@annelaerkes So, instead of: new_unit = ctsm_get_info(info$determinand,keep,"unit",info$compartment,sep="_") we would have new_unit = ctsm_get_info(info$determinand, keep, "unit", info$compartment, sep = "_")

annelaerkes commented 5 months ago

@RobFryer I think I got as far as I can by myself. I need help with the check of "weights" versus "drop"names, other than that I think it is working. Can we find a time to look through this part together? Except Wednesday, I should be flexible most other times this week

RobFryer commented 5 months ago

@annelaerkes @mml-BIOS-au

I've been tidying up the coding for the TEFs (deprecating determinand.link.TEQDFP and associated unit issues, and paving the way for the removal of info_TEF$DFP_environmental and info_TEF$DFP_human_health).

I was intending to set up a new element in info_TEF called simply info_TEF$DFP for the 2005 WHO TEFs. But then I came across The 2022 world health organization reevaluation of human and mammalian toxic equivalency factors for polychlorinated dioxins, dibenzofurans and biphenyls. So now I am thinking of adding two elements info_TEF$DFP_2005 and info_TEF$DFP_2022 instead. Does this seem sensible to you?

But looking at these papers again has prompted two further questions:

annelaerkes commented 5 months ago

@RobFryer Interesting with the new TEFs, I didn't know of these until now. I think it makes sense to include them as an option as they seem to be the new official WHO recommandations and they are based on a larger number of datasets. So, I guess over time (unless the OSPAR group finds some fault with these new recommandation) we will phase out the old ones and use these.

Regarding your other questions:

RobFryer commented 5 months ago

@annelaerkes @mml-BIOS-au

Many thanks. I'll ensure the full set of compounds are included in DFP_2005 and TEF_2022. I don't know why they were missing from the OSPAR and HELCOM calculations up until now.

I suspect we three are the only ones using the TEFs right now, so I might take this opportunity to relabel TEF_human_health as TEF_HOLAS3, since these were the values used for that assessment. And TEF_environmental as TEF_CEMP as these have been used in the CEMP assessment (up to 2024). I'll make sure these are documented in the help file that goes with info_TEF.

We (OSPAR) can agree to switch to the full set of 2005 TEFs at MIME 2024. Or indeed to the full set of 2022 TEFs, although maybe the threshold will need to be recalculated?

annelaerkes commented 5 months ago

@RobFryer @mml-BIOS-au I hadn't even considered that the EQS are of course calculated based on the TEQ sum and therefore linked to the WHO-2005. In the EQS draft for the new EQSs from January 2023 (which is the newest I have) they are not included, but one can only hope there has been some coordination so that the WHO-2022 TEFs will be used for the final version of the new TEQ EQSs (although probabaly not!)

RobFryer commented 5 months ago

@annelaerkes @mml-BIOS-au Hopefully this is the last question on this topic! There are two codes OCDF and CDFO that have got confused in the past, and now would be a good time to get them right. According to ICES RECO:

To my naive chemistry mind, it looks like we should have been using OCDF all along (rather than CDFO). (The ICES codes are hopelessly inconsistent, which is a large part of the problem.)

Should I make the switch in info_TEF at least for DFP_2005 and DFP_2022?

The downside is that most people are now submitting against CDFO (because we thought that was the correct code). We can get round that in the assessment by replacing all instances of CDFO with OCDF in determinands.control, and by encouraging new submissions and legacy submissions to use OCDF in future.

What do you think?

annelaerkes commented 5 months ago

@RobFryer @mml-BIOS-au I won't be of much help here since we are using different codes for most substances nationally than at ICES (see below). But I agree that "OCDF" seems to be more correct when looking at other publications as well, so it makes intuitive sense to me to use that moving forward. info_TEF <- c( "TCDD" = 1, "PECDD" = 1, "HXCDD1" = 0.1, "HXCDD2" = 0.1, "HXCDD3" = 0.1, "HPCDD" = 0.01, "OCDD" = 0.0003, "TCDF" = 0.1, "PECDF1" = 0.03, "PECDF2" = 0.3, "HXCDF1" = 0.1, "HXCDF2" = 0.1, "HXCDF3" = 0.1, "HXCDF4" = 0.1, "HPCDF1" = 0.01, "HPCDF2" = 0.01, "OCDF" = 0.0003)

RobFryer commented 4 months ago

@annelaerkes I'm just about to do a pull request for this. It was quite fiddly! I've asked you to review, but don't worry if you don't have time before you go on holiday.

And have a wonderful holiday + bike ride!

RobFryer commented 3 months ago

This has been resolved in #474. I will open a new issue to remove the deprecated link function in the release after next.