Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Smol updates #330

Merged
merged 5 commits into from
Aug 21, 2023
Merged

Smol updates #330

merged 5 commits into from
Aug 21, 2023

Conversation

ehinman
Copy link
Contributor

@ehinman ehinman commented Aug 18, 2023

  • updates following NP discussion, plus some small figure changes

- adds up whatever is present in each equation, do not need all subspecies
- if one subspecies present, will use that to provide total n measure
- shows subspecies used to get to total in TADA.NutrientSummationEquation column
@codecov-commenter
Copy link

codecov-commenter commented Aug 18, 2023

Codecov Report

Merging #330 (f5578ef) into develop (92e1601) will increase coverage by 0.07%.
The diff coverage is 3.70%.

❗ Current head f5578ef differs from pull request most recent head d55d992. Consider uploading reports for the commit d55d992 to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@             Coverage Diff             @@
##           develop     #330      +/-   ##
===========================================
+ Coverage    55.17%   55.24%   +0.07%     
===========================================
  Files           11       11              
  Lines         2784     2784              
===========================================
+ Hits          1536     1538       +2     
+ Misses        1248     1246       -2     
Files Changed Coverage Δ
R/Figures.R 0.00% <0.00%> (ø)
R/Transformations.R 53.43% <0.00%> (+2.42%) ⬆️
R/GenerateRefTables.R 87.67% <100.00%> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

I realized that if the equation accepts any combination of subsets of subspecies, other equations later in the hierarchy might be completely skipped even though a more complete set of subspecies exist. To fix this, I had to basically give the function every possible combination of nutrient subspecies, in a larger hierarchy. This is now saved as a .csv in the package.
@ehinman
Copy link
Contributor Author

ehinman commented Aug 19, 2023

I realized that if the equation accepts any combination of subsets of subspecies, other equations later in the hierarchy might be completely skipped even though a more complete set of subspecies exist to evaluate a later equation. To fix this, I had to basically give the function every possible combination of nutrient subspecies summations, in a larger hierarchy, and have each loop look for situations where ALL subspecies are present. This is now saved as a .csv in the package (NP_equations.csv).

For example, if a nutrient grouping on a single day/site had NITRATE and AMMONIA, because the TKN + NITRATE + NITRITE equation comes earlier in the hierarchy, the function would accept ONLY NITRATE to represent TOTAL N, when really it should skip down to the AMMONIA + NITRATE + NITRITE equation and work with those subspecies together. This is now fixed, and it will sum AMMONIA and NITRATE, as intended.

@cristinamullin cristinamullin merged commit bd6450c into develop Aug 21, 2023
6 checks passed
@cristinamullin cristinamullin deleted the smol_updates branch August 21, 2023 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants