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

combining arrays from different matrices does not seem to work (as I would expect) #10

Open
mfastudillo opened this issue Jul 14, 2023 · 1 comment

Comments

@mfastudillo
Copy link

In this stackoverflow question I try to exemplify the case. https://stackoverflow.com/questions/76632696/use-interfaces-for-scenario-analysis-in-brightway

If I create a combinatorial datapackage with one array changing values in the technosphere matrix (two possible values for one exchange) and other array changing values in the biosphere matrix (100 possible values for one exchange) I don't get the number of combinations that I would expect (200). However, if both arrays modify the same matrix (technosphere) I get the number of expected combinations.

@cmutel Perhaps this is the expected behaviour and this is not an issue but it looks strange to me

@cmutel cmutel transferred this issue from brightway-lca/bw_processing Jul 25, 2023
@cmutel
Copy link
Member

cmutel commented Jul 25, 2023

@mfastudillo Thanks for this feedback. The current implementation initializes one indexer per matrix, which indeed breaks your expectation. As I understand it, you want combinatorial logic to apply across all matrices when a datapackage specifies combinatorial=True.

This seems reasonable; the current behaviour is not going to change, as that would break existing code, but we can add an option to the LCA class init to share indexers across matrices. This would require re-initializing the combinatorial indexer, but this is fine.

The current behaviour applies combinatorial (and other) indexers per datapackage - I think this is also what we would want in a common indexer shared across matrices. People who want more complicated behaviour can override the indexers manually.

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

No branches or pull requests

2 participants