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

Reference flows equal to 0 aren't allowed to calculate anymore. #1170

Merged
merged 82 commits into from
Dec 20, 2023

Conversation

marc-vdm
Copy link
Member

It was not allowed for the sum of reference flows in a calculation setup to be equal to 0. This was an error.

AB should instead check for reference flows that are 0 themselves (needing 0 of something will by definition have 0 impact, so should not be allowed in calculation).

AB now checks for any ==0 reference flows and shows an appropriate error message informing user.

Checklist

  • Keep pull requests small so they can be easily reviewed.
  • Update the documentation, please follow the numpy style guide.
  • Update tests.
  • Categorize the PR by setting a good title and adding one of the labels:
    bug, feature, ui, change, documentation, breaking, ci
    as they show up in the changelog.
  • Link this PR to related issues by using closing keywords.
  • Request a review from another developer.

marc-vdm and others added 30 commits September 26, 2023 09:29
How to install using Mamba
…es (LCA-ActivityBrowser#1069)

* Updates to the functions from de Koning (get_relevant_flows and get_relevant_activities), avoiding use of pandas apply and using python map functionality for splitting pandas dataframes.

* Update the use of DataFrame.applymap to DataFrame.map in the excel file importer module.
* Updates to the logging system to avoid the print statement. Merging with
commits for corrections to the uncertainty distributions that were
included within the same branch. Improves thread safety in logging
increasing stability in the multi-threaded processes.

* Changes the creation of log files. Amends the uncertainty wizard test to correct for use of logging over the standard console. Adds the .logs to .gitignore.

* 1) Simplifies the use of the python logging facility with QtThreads (as published on the python docs for logging), improving the stability of logging.\n2) Changes the generation of the logging object that is more compatible with pytest, supporting the existing test procedures.

* Provides an update to the Model for the calculation_setup Methods table, corrects for deletion of impact assessment methods. Updates the routine to the lca_setup module for calling the logger.

* Updates to the logger module, what was the ABLogger class is renamed to ABHandler. The module now contains the formats and settings for the Stream handlers. Calling of the methods in the other AB modules requires passing a logging.Logger instance and the name of the calling module. These are then incorporated into the wrapping routines in the class. Additions:\n 1) An error wrapper is provided and also explicitly provides a trace of the error.\n 2) A timestamp routine is provided and used for providing the file names for log files.\n 3) A standard location based on appdirs is provided and used for log file locations.

* Corrections to the setup of the logger in the test_uncertainty_wizard module

* Includes a change to the type used for the scenario columns when using multiple files with the combined (combinatoric) approach for the scenarios. Includes respective changes to the boolean tests applied to such Indexes.

* Minor corrections to local repository branch, to keep changes aligned

* Alterations to keep minor changes aligned with master branch and fork

---------

Co-authored-by: zoo <[email protected]>
This was added before brightway2 supported multi-arch builds on
conda-forge. It is not needed anymore, because the normal AB is now also
installable on arm arch.
* Increase timeout to 30 min

Originally the idea was that installation should never take longer than
12 min, otherwise we should get a warning with the failed pipeline. But
unfortuantely conda currently takes longer than 12 minutes to solve
the environment.

* Add mamba install canary

* Download artifacts to start implementing env comparison

* Looks like on linux it can take more than 30 minutes :-(

* 60min not enough on linux, increasing to 120

* Completely remove timeout, default is 6 hours

* Use solver libmamba option

* Add diff step to compare installations

* Use node16 actions

* Re-add the 12 timeout

* Split diff into separate steps

* Yq action only runs on linux

* debug yq formatting step

* Simplify

* more fighting with yq action

* artifacts are apparently directories in this case

* Try again with while loop

* Run on all os, but only 3.9

* ignore diff exit code

* Run canary install for 3.8 and 3.9 again

---------

Co-authored-by: haasad <[email protected]>
mrvisscher and others added 21 commits December 4, 2023 20:18
* Switch project on delete LCA-ActivityBrowser#1075

* Cleaning up ProjectController.change_project

* Resolving comments by marc-vdm
GH action script for commenting on issues when related milestone is c…
GH action script for commenting on issues when related milestone is c…
GH action script for commenting on issues when related milestone is c…
Automated review instructions
@marc-vdm marc-vdm added the bug Issues/PRs related to bugs label Dec 17, 2023
@marc-vdm marc-vdm added this to the 2.9.3 milestone Dec 17, 2023
@coveralls
Copy link

coveralls commented Dec 17, 2023

Coverage Status

coverage: 50.392% (+0.06%) from 50.33%
when pulling 04dd246 on marc-vdm:calculation_setup_0_msg
into 51566ac on LCA-ActivityBrowser:master.

@marc-vdm marc-vdm merged commit 66a0227 into LCA-ActivityBrowser:master Dec 20, 2023
12 checks passed
Copy link

This issue has been implemented in the new release of Activity Browser 🚀 (version 2.9.3), you can get the new version by updating Activity Browser.

🤖beep boop! I'm a bot and this message was an automated action. If updating does not make sense for this issue, just ignore this.

@marc-vdm marc-vdm deleted the calculation_setup_0_msg branch May 14, 2024 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues/PRs related to bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants