brightway-lca / bw_timex

Time-explicit Life Cycle Assessment
https://docs.brightway.dev/projects/bw-timex
BSD 3-Clause "New" or "Revised" License
27 stars 2 forks source link

datetime calculations + aggregation can lead to wrong timestamp #80

Open muelleram opened 1 month ago

muelleram commented 1 month ago

With a relative TD of minus 1 and 2 years between the producer B and the consumer A: "temporal_distribution": TemporalDistribution( np.array([-2, -1,], dtype="timedelta64[Y]"), np.array([0.5, 0.5]), ), and a starting_datetime at node A of 2024-01-01, we get the following edge_timeline: image

The -2 and -1 years are converted to 2021-12-31 and 2021-12-31, respectively because of the underlying calculations in seconds and the duration of a year in datetime.datetime. These timesteps, then, converted to the year 2021 and 2022 (as we convert to the beginning of the temporal_grouping unit), while it should be converted to the year 2022 (two years before 2024) and 2023 (one year before 2024).

Maybe we could use relativedelta or similar from dateutil (http://labix.org/python-dateutil).

Not a high priority but should be eventually looked at.

TimoDiepers commented 1 month ago

@muelleram shouldn't we just round to the nearest year (or general temporal grouping) instead of cutting it off?

muelleram commented 1 month ago

yes, that could be a pragmatic way, as well. Or maybe add a threshold (e.g. 2% of the time span of the temporal grouping) to catch those edge cases, when it's about gap years, amount of days in a month, etc. No strong preference from my side.