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

Ghosh implementation for downstream scope 3 #136

Merged
merged 6 commits into from
Apr 2, 2024

Conversation

Beckebanze
Copy link
Contributor

See issue #135 for more details.

With this PR the following is added/implemented:

  • equivalent of A for Ghosh (often referred to A* in literature)
  • the Ghosh inverse (often referred to G in literature). If the Leontief matrix L is already calculated, then G is calculated based on L because the calculation based on A* is computationally much more expensive.
  • downstream scope 3 multiplier, M_{down}, such the sum of the M+M_{down} is the full scope multiplier, with M the existing multiplier in pymrio that covers scope 1,2&3 upstream.
  • a short addition to the pymrio background page that introduces the Ghosh model
  • tests that test the functionality of the added functions

@konstantinstadler
Copy link
Member

konstantinstadler commented Feb 27, 2024

Hi, there are some problems with pytest 8.*, can you change the line in the environment.yml file to
- pytest >= 5.4.3, < 5.8.0
and then push again to the pr

Exclude pytest 5.8.0 because this version has an issue.
@Beckebanze
Copy link
Contributor Author

Thanks. I updated the environment.yml.

@konstantinstadler
Copy link
Member

So the test run now, just one small problem with the formating. When you run the format_and_test from the route within your environment it should just format everything as it should be. If you are not on linux, you can run the lines of the files from the cml in windows.

I run it locally, and then all your tests pass

@Beckebanze
Copy link
Contributor Author

Indeed. iomath.py was not yet correctly formatted in this PR. It apparently slipped through when I committed all formatting changes to my development branch.

@konstantinstadler
Copy link
Member

There is still some black problems, do you use the same version ( black 24.2.0 ).. See also the details of the run

@konstantinstadler
Copy link
Member

@Beckebanze : let me know if there are any issues

@Beckebanze
Copy link
Contributor Author

Beckebanze commented Mar 19, 2024 via email

@konstantinstadler
Copy link
Member

Hi,

If you go into the CI run you see a section "list environment".
There you find the versions of all packages used, for black:
black 24.2.0 py39hf3d152e_0 conda-forge

Your changes pass the tests on my local machine as well - so yes, I guess it is just some annoying version conflict.. I am kind of pushing you to do that via a pull request so you are listed as an official contributor.
But for now enjoy your holiday!

…s that are not related to the functionality added with this PR.
@Beckebanze
Copy link
Contributor Author

Beckebanze commented Mar 27, 2024 via email

Copy link
Member

@konstantinstadler konstantinstadler left a comment

Choose a reason for hiding this comment

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

Great, I will merge that for now, but we need some more documentation on the use

@konstantinstadler konstantinstadler merged commit a9896ff into IndEcol:master Apr 2, 2024
13 checks passed
@konstantinstadler
Copy link
Member

Fantastic to get that in!

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.

None yet

2 participants