brightway-lca / brightway2-calc

The calculation engine for the Brightway2 life cycle assessment framework.
BSD 3-Clause "New" or "Revised" License
12 stars 15 forks source link

Interface design for multiple LCIA methods #30

Closed cmutel closed 4 months ago

cmutel commented 4 years ago

It would be sensible for multiple LCIA methods to be calculated at once - however, it is unclear exactly what this would look like. What does it look like to the end user? For example, is LCA.method now LCA.methods? Is LCA.characterization_matrix a list? What about LCA.score(s)?

One option would be for all method attributes to become lists, but behave like a single element if no explicit index is provided. However, this might be a bit too much magic. On the other hand, a switch from LCA.method to LCA.methods (which would always be a list) will break all existing Brightway code. So maybe the LCA class should always only support one impact category, and a new class should be added for multiple methods and functional units?

I don't know. Please provide your opinions, and, more helpfully, some actual interface ideas!

massimopizzol commented 4 years ago

I use a for loop or similar to calculate for multiple FUs and ICs so this feature seems a bit redundant. My preference is thus for: "the LCA class should always only support one impact category, and a new class should be added for multiple methods and functional units" to let the user more freedom.

mfastudillo commented 4 years ago

I don't see it as fundamental either. So I would not break many things to make it work. I think a child class with some wrapping around switch_method to support multiple scores would be nice to have

bsteubing commented 4 years ago

I don't see this as a big issue either. An MLCA class (using calculation setups as the input) is already available in the Activity Browser. https://github.com/LCA-ActivityBrowser/activity-browser/blob/master/activity_browser/app/bwutils/multilca.py

So my vote would be for keeping it as it is.

However, I do understand that many people simply want to do LCIA for all (e.g.) ReCiPe (H) impact categories. Perhaps there is a way to facilitate the selection of these methods by also providing "method groups" or something similar, where the user would simply have to choose "ReCiPe (H) 2016" (which would then be the list of all sub-methods).

tngTUDOR commented 4 years ago

(Right now, I'm more comfortable using a calculation setup with multiple lcas.)

Maybe the underlying question is: should "method" refer to "[1] (the list of) all tuples identifying a method where the first element is identical or to [2] a unique tuple?" Right now, an instance of the Method class is identified through a unique tuple. I have the feeling that the method "concept" in real life, does not match the class Method. Correct me if this is only a hunch, but I believe that people tend to think that method should refer to all the different impact categories of a methodology.

tngTUDOR commented 4 years ago

My suggestion is break things only for a full major version (break them for BW3!) and for 2.5:

  1. create a new class that uses methods as a plural or uses a suffix [or preffix]
  2. deprecate LCA class
  3. add some warnings from now on, like pandas did before the release of 1.0 releases of the deprecation

I suggest we keep things in the spirit of the Activity class, where we have exchanges()

Right now, I can only think of the following names for the new class:

Another option to not break existing functionality, while adding multiple lcia methods to an LCA would be to add the "group" suffix (or prefix) to current method, matrix, and score properties of the LCA:

an_lca = LCA(an_activity, method_group =[('m1', 'c', 's1'), ('m2', 'c', 's2')])
...
the_scores = an_lca.score_group
...

Of course, group is only a synonym of list, but there are a few other [funnier and more respectful] : chain, bundle, pool, assortment, batch.

bsteubing commented 4 years ago

(Right now, I'm more comfortable using a calculation setup with multiple lcas.)

Maybe the underlying question is: should "method" refer to "[1] (the list of) all tuples identifying a method where the first element is identical or to [2] a unique tuple?" Right now, an instance of the Method class is identified through a unique tuple. I have the feeling that the method "concept" in real life, does not match the class Method. Correct me if this is only a hunch, but I believe that people tend to think that method should refer to all the different impact categories of a methodology.

I agree with this. "Method" is a very fuzzy term. Eutrophication is an "impact category", but it is modelled within several impact assessment method families (I don't even know the correct term for the latter). These concepts are currently implicitly represented by tuples such as ('ReCiPe X', 'climate change', '100a') in brightway and the beauty of this concept is that it is very flexible (any number of items in this tuple, e.g. to add "biogenic" as a 4th item). Finding the right mix between flexibility and practical relevance is key here, hence the previous idea of adding "method groups" or families as a "convenience layer" that would simply group established methods (e.g. all ReCiPe (H) impact categories) (maybe like suggested by @tngTUDOR, although I would think that I'd prefer to modify it as shown below: )

an_lca = LCA(an_activity, method_group =['ReCiPe (H)'])

where the associated "methods" are identified from a dictionary provided within brightway:

method_groups = {
     'ReCiPe (H)': [('m1', 'c', 's1'), ('m2', 'c', 's2')]
     ...
}
tngTUDOR commented 4 years ago

@bsteubing we could have any kind of generator or list comprehension in the method_group ;)

an_lca = LCA(an_activity, method_group =[m for m in methods if m[0] == "My Method"])
PascalLesage commented 4 years ago

I agree with @tngTUDOR that things should be broken only for v3, with deprecation warnings now if things are meant to be broken in the future. While I am also quite at ease using for loops, and with loading and using multiple CF matrices using dictionaries or lists, I can see how having convenience functions that would do all that for the user can be useful. I think that if something had to be done, I would try to keep the LCA class as streamlined as possible, with methods for collecting basic matrix loading and manipulations, and would try to use child classes to extend functionality, as was mentioned above (e.g. MultiMethodLCA or something of the sort). Perhaps having LCA class accept cf matrices as arguments (rather than just method name) could help. Right now, you can't use LCA.lcia unless you've passed an actual method, and the matrix loading needs to occur within LCA.

BenPortner commented 4 years ago

Here is my opinion: 1) extend the multi_lca class signature by two optional arguments methods and demands that can be provided alternatively to cs_name (I cannot implement this right now because there is a call to function wrap_functional_unit, which is not implemented in the latest version on GitHub) 2) extend the current LCA constructor by a check: if method and/or demand are of type list then make a multi_lca object and do the calculations there. If method is a tuple and demand a dict then proceed as implemented currently 3) I agree with @bsteubing and @tngTUDOR that users might want to process all scores belonging to a method group. A method group could be a tuple with a single entry, e.g. ('ILCD 2.0 2018 midpoint'). Passing such a tuple should return all scores of the method group as a dict or numpy array.

@PascalLesage: You should be able to pass a matrix without defining a method as follows:

lca = LCA({act:1})
lca.lci()
lca.characterization_matrix = your_matrix
lca.lcia_calculation()
print(lca.score)
cmutel commented 11 months ago

Thanks everyone for your inputs. I am going to try working with the following approach:

cmutel commented 11 months ago

Big step in matrix_utils, released in 0.3: We can now feed a dictionary of data packages to create a dictionary of mapped matrices with a unified interface. This means they have a shared indexer, the same dimensions, etc.

My new conception is that we need to provide labeled inputs for both the impact categories (and weightings, and normalizations), and the demands. This breaks compatibility with the existing calculation setup, but in the end it is much more sensible to refer to results for ("trucks eff class B", ("IPCC", "GWP100")) than use magic index numbers or something else without real meaning.

cmutel commented 4 months ago

The design is now fixed. See MultiLCA and MethodConfig object.