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

fix company level sda targets #446

Merged
merged 6 commits into from
Sep 6, 2023
Merged

fix company level sda targets #446

merged 6 commits into from
Sep 6, 2023

Conversation

jacobvjk
Copy link
Member

@jacobvjk jacobvjk commented Sep 1, 2023

closes #445

This PR:

  • ensures that target_sda(by_company = TRUE) uses the emission intensity value of the final year of the adjusted_scenario_* to calculate the target_* values for companies when using the SDA approach
  • this ensures that - regardless of whether the SDA is applied at company or loan book level - the individual scenario targets converge at the correct scenario value
  • the issue had been caused by a right_join, that introduces NAs for every year past the emission intensity projection when running target_sda() at the company level.
  • some implementation details:
    • previously, target_summary_groups was adjusted to loan book or company level at the top level of target_sda()
    • it is moved into compute_loanbook_targets(), which gains an argument, by_company, to adjust the grouping variables as needed.
    • the former functionality in compute_loanbook_targets() was one pipe, which is now split into multiple parts.
    • the first part is only the right_join that was previously used too
    • the latter part of the pipe also stays intact
    • we add an intermediate step that is triggered in case targets are calculated on the company level. This optional step ensures the adjusted scenario_with_p values are correctly merged for every individual company, ensuring the relevant convergence value exists when calculating company targets
  • the snapshot for the company level target_sda() calculation is updated, as it previously contained NAs and wrong values due to this bug

@jacobvjk
Copy link
Member Author

jacobvjk commented Sep 1, 2023

  • checks pass locally
  • failing checks are likely due to the same reason mentioned by @jdhoffa here:

[Created on 2023-09-01 with reprex v2.0.2](#442 (comment))

@jacobvjk jacobvjk marked this pull request as ready for review September 6, 2023 13:08
Copy link

@Antoine-Lalechere Antoine-Lalechere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jacobvjk jacobvjk merged commit 5f603ef into main Sep 6, 2023
10 of 23 checks passed
@jacobvjk jacobvjk deleted the fix-company-sda branch September 6, 2023 16:23
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.

Convergence target wrong in target_sda(by_company = TRUE)
4 participants