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 amounts and add tests #12

Closed
TimoDiepers opened this issue Jan 25, 2024 · 6 comments
Closed

Fix amounts and add tests #12

TimoDiepers opened this issue Jan 25, 2024 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@TimoDiepers
Copy link
Member

TimoDiepers commented Jan 25, 2024

The amounts just happened to work out because some exchange amounts were set to 1, so the interpolated shares had no influence on the total. We should also add tests checking if the Medusalca calculations are correct.

@TimoDiepers TimoDiepers added the bug Something isn't working label Jan 25, 2024
@TimoDiepers TimoDiepers self-assigned this Jan 25, 2024
@TimoDiepers TimoDiepers changed the title Fix interpolated amounts in matrix and add tests Fix amounts and add tests Jan 25, 2024
@TimoDiepers
Copy link
Member Author

TimoDiepers commented Jan 25, 2024

  • Fixed amounts
  • Added tests

@TimoDiepers
Copy link
Member Author

TimoDiepers commented Jan 25, 2024

Amounts fixed in 23ea70c

The problem was that we had to use the "unscaled" amounts from the original edge here, as we explode to new unit processes. In contrast, the amounts from the timeline are scaled to match the FU.

@TimoDiepers
Copy link
Member Author

Additional problem came to my attention:
The setting: A foreground node (H2) links to a background node (el. mix) via a TD.
image

What happens in our code atm is that we don't link from H2 to exploded versions of el-mix, but link directly to the different available version of this process in the background dbs via interpolation. However, if two of these interpolations point towards the same background process, the matrix entry we add gets overwritten each time.

I'm pretty sure I talked about this with @cmutel at the Autumn School and he said that the default behavior of Datapackage.add_persistent_vector is to add the values instead of overwriting them - which seems to not be the case.
@jakobsarthur do you happen to know if changing this behavior is possible? Going through Datapackage.add_persistent_vector() I couldn't find what we need..

@TimoDiepers
Copy link
Member Author

Nvm, found the sum_inter_duplicates flag in bwp.create_datapackage()

@TimoDiepers
Copy link
Member Author

From the datapackage-creation-side, this should be fixed in b84644f
However, the results are still wrong. But I think the problem might be in the timestamps in the timeline df.

@jakobsarthur
Copy link
Collaborator

Great! I will check the timestamps of the timeline. Still working on that, also in issue #5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants