Migrated from miniconda to official python bullseye docker image

[santacodes]

Description

This draft is in continuation to #3874 which was closed due to messy branches and the image failing to build IDAKLU solver on aarch64 linux. Initially the idea was to migrate to a single docker image of manylinux2014 which was compliant to PEP599, but due to some complications, the official python image along with venv were decided upon implementation, check #3874 discussions for more information.

Fixes #3692
Fixes #3666

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

• New feature (non-breaking change which adds functionality)
• Optimization (back-end change that speeds up the code)
• Bug fix (non-breaking change which fixes an issue)

Key checklist:

• No style issues: $ pre-commit run (or $ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)
• All tests pass: $ python run-tests.py --all (or $ nox -s tests)
• The documentation builds: $ python run-tests.py --doctest (or $ nox -s doctests)

You can run integration tests, unit tests, and doctests together at once, using $ python run-tests.py --quick (or $ nox -s quick).

Further checks:

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[agriyakhetarpal]

Thanks @santacodes, happy to keep this as a draft for now for visibility. xref #3879 which needs to be resolved before we are able to come back to this.

[cringeyburger]

Hey, @santacodes! I wanted to ask if you could finish the pull request.
I need to make some changes to the Dockerfile to adjust the build commands, but since we're planning to move away from miniconda, it would be great if you could finalize this PR and get it merged.
This will allow the migration PR to move forward smoothly.

@arjxn-py has also agreed to help review the PR.
Thanks a lot!

[santacodes]

Hey, @santacodes! I wanted to ask if you could finish the pull request. I need to make some changes to the Dockerfile to adjust the build commands, but since we're planning to move away from miniconda, it would be great if you could finalize this PR and get it merged. This will allow the migration PR to move forward smoothly.

@arjxn-py has also agreed to help review the PR. Thanks a lot!

Sure I'll get on it!

[santacodes]

@arjxn-py do you mind if I change the standard pip installations of packages to uv? Could it possibly reduce the build times?

[arjxn-py]

I don't mind at all @santacodes, feel free to switch to uv. We also have an open issue for the same #3825, this would be a good step in that direction. Thanks @cringeyburger for following up.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.46%. Comparing base (fa34276) to head (22e5178).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3901   +/-   ##
========================================
  Coverage    99.46%   99.46%           
========================================
  Files          289      289           
  Lines        22146    22146           
========================================
  Hits         22027    22027           
  Misses         119      119           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[arjxn-py]

You'd also want to set the docker workflow to temporarily run on this PR

[santacodes]

Also while running unit tests inside the docker container it prompted this -

SKIPPED [1] tests/unit/test_solvers/test_idaklu_jax.py:91: Both IDAKLU and JAX are available
SKIPPED [1] tests/unit/test_util.py:91: The JAX solver is not installed

I don't know if it has been addressed already but I see the same thing on the CI as well, I think this part of code in tests/unit/test_util.py is causing it, instead of "The JAX solver is not installed" it should be "The JAX solver is installed". Correct me if I am wrong 😅 .

@pytest.mark.skipif(pybamm.have_jax(), reason="The JAX solver is not installed")
    def test_is_jax_compatible(self):
        assert True

[arjxn-py]

Yes currently it skips if pybamm.have_jax(), which should not happen.
I should have been a mistake while #4214, see here
Feel free to modify this, there should not be a condition to skip and it should check for pybamm.is_jax_compatible(). Hope i'm not missing anything.

[santacodes]

Yes currently it skips if pybamm.have_jax(), which should not happen. I should have been a mistake while #4214, see here Feel free to modify this, there should not be a condition to skip and it should check for pybamm.is_jax_compatible(). Hope i'm not missing anything.

Should I just fix it in this PR or make a new one? Also, the assert inside the function should be False if I am not wrong?

[arjxn-py]

I noticed the workflow failed because of a login error, should

Feel free to skip login & switch to cli build command temporarily:

docker buildx build \
  --file scripts/Dockerfile \
  --tag pybamm/pybamm:latest \
  --push \
  --platform linux/amd64,linux/arm64 \
  .

if that works.

Should I just fix it in this PR or make a new one? Also, the assert inside the function should be False if I am not wrong?

assert should be true after pybamm.is_jax_compatible() as if is_jax_compatible() returns false, it should fail. Separate PR should be good as it's unrelated to this one.

[kratman]

@santacodes Just an FYI, force pushing removes the history of what was reviewed. Try not to force push after review has started

[santacodes]

@santacodes Just an FYI, force pushing removes the history of what was reviewed. Try not to force push after review has started

Sorry my commits got messed up between 2 branches, so I had to rebase them 😅 will keep that in mind!

[arjxn-py]

Thanks @santacodes