Issue when starting AB - Githubissues

AJue3101 commented 2 years ago

Hi,

I got this Problem when starting AB. The error had also occurred before, but downgrading bw2calc to version 1.8.0 had always fixed the error. And I have always worked with a Python version >=3.8 (now with 3.9.10)

(ab) PS C:\Users\Snickers> activity-browser

Traceback (most recent call last): File "C:\Users\Snickers\miniconda3\envs\ab\Scripts\activity-browser-script.py", line 5, in from activity_browser import run_activity_browser File "C:\Users\Snickers\miniconda3\envs\ab\lib\site-packages\activity_browser__init.py", line 9, in from .application import Application File "C:\Users\Snickers\miniconda3\envs\ab\lib\site-packages\activity_browser\application.py", line 2, in from .controllers import controllers File "C:\Users\Snickers\miniconda3\envs\ab\lib\site-packages\activity_browser\controllers__init__.py", line 2, in from .activity import ActivityController, ExchangeController File "C:\Users\Snickers\miniconda3\envs\ab\lib\site-packages\activity_browser\controllers\activity.py", line 5, in import brightway2 as bw File "C:\Users\Snickers\miniconda3\envs\ab\lib\site-packages\brightway2\init__.py", line 3, in from bw2calc import * AttributeError: module 'bw2calc' has no attribute 'ComparativeMonteCarlo'

However, it doesn't work now - I get the error message again: AttributeError: module 'bw2calc' has no attribute 'ComparativeMonteCarlo'. The same happens with the stable version

haasad commented 2 years ago

Hi @AJue3101, I just opened https://github.com/brightway-lca/brightway2-calc/issues/43 to report the problem, I'm seeing the same issue

haasad commented 2 years ago

You can try downgrading scipy to an older version, I suspect that scipy API changes are currently breaking brightway2:

conda install scipy=1.7

AJue3101 commented 2 years ago

Hi @haasad, thanks for the quick feedback - downgrading scipy works

marc-vdm commented 2 years ago

@haasad, thanks for being on top if this! Is this different from #703?

Can we fix this from our side by pinning scipy=1.7?

haasad commented 2 years ago

@marc-vdm Yes, it's different from #703 (even if the error message is the same). Scipy 1.8.0 was released yesterday and changed some APIs. Ideally this should be fixed upstream in bw2calc, but we could add a temporary pin for scipy until then. However somebody would need to address the failing tests, otherwise the release pipeline won't run through.

marc-vdm commented 2 years ago

However somebody would need to address the failing tests, otherwise the release pipeline won't run through.

@nabilahmed739 @bsteubing Could we have a look at the testing stuff soon? I want to learn how to fix these issues.

bsteubing commented 2 years ago

@marc-vdm @nabilahmed739 @haasad I have fixed 2 of the reasons why the tests were failing:

changes I made in https://github.com/LCA-ActivityBrowser/activity-browser/commit/bf0b957630e7b5b58f1da9cd8aa66052196e1e96 which caused tests in test_presample_scenarios.py to fail (which I commented for now)
conda package incompatibility ... for now I pinned scipy < 1.8 https://github.com/LCA-ActivityBrowser/activity-browser/commit/8d9f44ada0f4ef363d3427af108f8b45695c6963 (we could test also pinning bw2calc to 1.8.1, but there was previously some reason why we had bw2calc = 1.8.0; happy if someone could take this on)

Yet there is another reason to do with matplotlib on MacOS that I fail to understand... https://github.com/LCA-ActivityBrowser/activity-browser/runs/5127971434?check_suite_focus=true

Solutions are appreciated...

haasad commented 2 years ago

@bsteubing please review #711, I'll open a separate issue for fixing the dependencies, I couldn't easily get bw2calc 1.8.1 to work

bsteubing commented 2 years ago

@haasad strange, in your pull request it worked, now after merging it does not work... https://github.com/LCA-ActivityBrowser/activity-browser/runs/5144321235?check_suite_focus=true It looks like we are again a step further as it passed for MacOS_38, but not for MacOS_39

haasad commented 2 years ago

I restarted the workflow twice, now it ran through :man_shrugging:

haasad commented 2 years ago

I skipped one more test: https://github.com/LCA-ActivityBrowser/activity-browser/commit/d6e29cc4b370da20d1f6105fd032ff3ab37a34bc

Really not sure what causes this, would need somebody with a Mac to run the tests locally. It's really hard to debug when you have to wait for a 10min pipeline run after every change.

marc-vdm commented 2 years ago

If there is anything I can do to help let me know.

I just tried to re-run tests on PRs that don't do anything critical but that still breaks. Both my PR #710 and #708 have problems. #710 fails macOS test for 3.9 and #708 fails all py3.9 tests. py3.8 tests seem to get cancelled, which I also don't understand?

Do I need to pull/merge the current master and push again for those new tests to work? The PR #711 was not merged into either branch yet as my PRs existed before we fixed the issue.

haasad commented 2 years ago

@marc-vdm Your PRs run against the github actions defined in your branch, not the master branch. I also find this a bit confusing, but at least it let's you test changes to the worfklows.

Not sure how comfortable you are with more "advanced" git operations, but this is how I would do it:

add the AB repo as upstream in your fork as described here: https://www.atlassian.com/git/tutorials/git-forks-and-upstreams
get changes from upstream: git fetch upstream
make sure you have your branch checked out locally: git checkout -b <your-feature-branch>
add the changes from upstream master with rebase: git rebase upstream/master
push your the changes to to the branch in your fork: git push --force
- you have to force push, because you're "rewriting" the history in your branch. But as long as you're the only person working on that branch, that's no problem. Overall this helps to keep the commit history cleaner
if you don't like the rebase workflow you can also git merge upstream/master

if you push the changes to your fork, the workflows should run again and (hopefully) succeed.

Regarding the cancelled tests: We could/should add strategy: [fail-fast: false] for the tests in the workflow (see e.g. here). Without it the pipelines stop if one step of the "matrix" fails, i.e. if 3.8 tests fail 3.9 are stopped and vice-versa.

marc-vdm commented 2 years ago

Thanks! I rebased (already had upstream in my pycharm) and and all the checks completed and passed. So the solution seems to work well.

bsteubing commented 2 years ago

I restarted the workflow twice, now it ran through 🤷‍♂️

Thanks @haasad ... seems like black magic at times... at least we have a running development version out now. I guess for the stable version I will wait until we have figured out the dependencies... or do you and @marc-vdm think the way this is solved is fine for now?

marc-vdm commented 2 years ago

These kinds of problems are honestly out of my comfort zone. I'm happy to help indicate or diagnose problems where I can, but I don't have the knowledge to fix them or tell you whether these fixes are good enough unfortunately.

cmutel commented 2 years ago

@haasad @marc-vdm @bsteubing This should be fixed in upstream bw2calc and brightway2 releases. Please close if you agree.

marc-vdm commented 2 years ago

Going to keep this open for a little longer (sorry all <3): @haasad this MacOs test failed twice, do I just keep re-running the test or is there a way to find if it is my code that is actually failing?

haasad commented 2 years ago

I opened #723 for the segfaulting tests on OSX. The original problem in this issue is addressed with #716 and #722

LCA-ActivityBrowser / activity-browser

Issue when starting AB #707