Skip to content

Commit

Permalink
Merge pull request #111 from KingsburyLab/apple_silicon
Browse files Browse the repository at this point in the history
Testing, documentation, and exception handling for Apple Silicon
  • Loading branch information
rkingsbury authored Mar 5, 2024
2 parents b9d8172 + b8b0705 commit 28ce5fc
Show file tree
Hide file tree
Showing 13 changed files with 148 additions and 28 deletions.
65 changes: 58 additions & 7 deletions .github/workflows/post-process.yml
Original file line number Diff line number Diff line change
@@ -1,19 +1,70 @@
name: Post-process
name: Post-merge

# triggered when PRs are merged into main branch
on:
workflow_run:
pull_request:
branches:
- main
types:
- completed
workflows:
# List all required workflow names here.
- 'testing'
- closed

jobs:
# https://github.com/marketplace/actions/tag-release-on-push-action
lint:
if: github.event.pull_request.merged == true
runs-on: ubuntu-latest
strategy:
max-parallel: 1
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: 3.11
cache: pip
- name: Run pre-commit
run: |
pip install pre-commit
pre-commit run
test-comprehensive:
needs: lint
strategy:
max-parallel: 6
matrix:
python-version: ["3.9", "3.10", "3.11", "3.12"]
os:
- ubuntu-latest
- macos-latest
- windows-latest
- macos-14
exclude:
- os: macos-14
python-version: "3.9"
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}${{ matrix.dev }}
- name: Install test requirements
run: |
python -m pip install --upgrade pip
pip install -e ".[testing]"
- name: Run tests
run: |
pytest --cov=src/pyEQL --cov-report=xml
- uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
file: ./coverage.xml

# https://github.com/marketplace/actions/tag-release-on-push-action
auto-gen-release:
name: Tag/Release on merge of labeled PRs
needs: test-comprehensive
runs-on: ubuntu-latest
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Expand Down
32 changes: 19 additions & 13 deletions .github/workflows/testing.yaml
Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@
name: testing

on:
push:
pull_request:
branches:
- main
paths-ignore:
- 'docs/'
- CHANGELOG.md

pull_request:
branches:
- main

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
Expand Down Expand Up @@ -40,18 +35,29 @@ jobs:
strategy:
max-parallel: 6
matrix:
python-version: ["3.9", "3.10", "3.11", "3.12"]
platform:
- ubuntu-latest
- macos-latest
- windows-latest
runs-on: ubuntu-latest
# for most PRs, test the min and max supported python on every platform, test all python on ubuntu
python-version: ["3.9", "3.12"]
os:
- ubuntu-latest
- macos-latest
- macos-14
- windows-latest
include:
- os: ubuntu-latest
python-version: "3.10"
- os: ubuntu-latest
python-version: "3.11"
# no python 3.9 on the macos-14 runner
exclude:
- os: macos-14
python-version: "3.9"
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python }}${{ matrix.dev }}
python-version: ${{ matrix.python-version }}${{ matrix.dev }}
- name: Install test requirements
run: |
python -m pip install --upgrade pip
Expand Down
22 changes: 19 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,33 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleased
## [0.14.0] - 2024-03-05

## [0.13.0] - 2024-03-05
### Added

- `NativeEOS` / `PhreeqcEOS`: Added `try`/`catch` so that `pyEQL` can still be used on platforms that PHREEQC does
not support, such as Apple Silicon. In such cases, functions like `equilibrate` that depend on PHREEQC will
raise errors, but everything else can still be used.
- CI: Added Apple M1 runner (GitHub: `macos-14`) to the CI tests.

### Fixed

- `equilibrium.alpha()`: Fixed incorrect calculation of acid-base distribution coefficient for multiprotic acids.
- CI: Addressed several issues in the testing configuration which had resulted in testing
fewer operating systems x python version combinations than intended. CI tests now
correctly and comprehensively test every supported version of python on every os
(macos, windows, ubuntu).
- `utils.FormulaDict`: implemented `__contains__` so that `get()` works correctly in
python 3.12+. See https://github.com/python/cpython/issues/105524
- Docs: fixed many small problems in documentation causing equations and examples to
render incorrectly.
- `Solution.from_file`: Add missing `@classmethod` decorator; update documentation.

## [0.13.0] - 2024-03-05

### Fixed

- `equilibrium.alpha()`: Fixed incorrect calculation of acid-base distribution coefficient for multiprotic acids.

## [0.12.2] - 2024-02-25

### Fixed
Expand Down
10 changes: 10 additions & 0 deletions docs/engines.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,16 @@ as needed to particular use cases.
`pyEQL` currently supports three modeling engines: `ideal`, `native`, and `phreeqc`, which
are selected via the `engine` kwarg to `Solution.__init__()`. Each engine is briefly described below.

```{warning}
If you are using a Mac with an Apple M1, M2, etc. chip (i.e., Arm64 architecture), some features of `pyEQL` will be
unavailable. Specifically, anything which depends on PHREEQC (e.g., the
`equilibrate` method in the native engine and the entire
[`phreeqc` engine](engines.md#the-phreeqc-engine)) will not work. This is because `phreeqpython` is currently
not available for this platform. All other functions of `pyEQL` should work as expected.
Feel free to post your experiences or proposed solutions at https://github.com/KingsburyLab/pyEQL/issues/109
```

## The `'native'` engine (Default)

The `native` engine is the default choice and was the only option available prior to
Expand Down
13 changes: 10 additions & 3 deletions docs/installation.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,29 @@ You can tell if this is the case by typing the following command:
```

```
$ python --version
Python 2.7.12

```

This means Python 2.x is installed. If you run 'pip install' it will point to the Python 2.7 installation, but pyEQL
only works on Python 3. So, try this:

```

$ python3 --version
Python 3.9.7

```

To get to Python 3.x, you have to type 'python3'. In this case, you would run 'pip3 install'

```{warning}
If you are using a Mac with an Apple M1, M2, etc. chip (i.e., Arm64 architecture), some features of `pyEQL` will be
unavailable. Specifically, anything which depends on PHREEQC (e.g., the
`equilibrate` method in the native engine and the entire
[`phreeqc` engine](engines.md)) will not work. This is because `phreeqpython` is currently
not available for this platform. All other functions of `pyEQL` should work as expected.
Feel free to post your experiences or proposed solutions at https://github.com/KingsburyLab/pyEQL/issues/109
```

## Other dependencies
Expand Down
10 changes: 9 additions & 1 deletion src/pyEQL/engines.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,15 @@ def __init__(
Path(os.path.dirname(__file__)) / "database" if self.phreeqc_db in ["llnl.dat", "geothermal.dat"] else None
)
# create the PhreeqcPython instance
self.pp = PhreeqPython(database=self.phreeqc_db, database_directory=self.db_path)
# try/except added to catch unsupported architectures, such as Apple Silicon
try:
self.pp = PhreeqPython(database=self.phreeqc_db, database_directory=self.db_path)
except OSError:
logger.error(
"OSError encountered when trying to instantiate phreeqpython. Most likely this means you"
" are running on an architecture that is not supported by PHREEQC, such as Apple M1/M2 chips."
" pyEQL will work, but equilibrate() will have no effect."
)
# attributes to hold the PhreeqPython solution.
self.ppsol = None
# store the solution composition to see whether we need to re-instantiate the solution
Expand Down
2 changes: 1 addition & 1 deletion src/pyEQL/solution.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def __init__(
if database is None:
# load the default database, which is a JSONStore
db_store = IonDB
elif isinstance(database, str | Path):
elif isinstance(database, (str, Path)):
db_store = JSONStore(str(database), key="formula")
logger.info(f"Created maggma JSONStore from .json file {database}")
else:
Expand Down
5 changes: 5 additions & 0 deletions src/pyEQL/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,5 +99,10 @@ def __setitem__(self, key, value):
# sort contents anytime an item is set
self.data = dict(sorted(self.items(), key=lambda x: x[1], reverse=True))

# Necessary to define this so that .get() works properly in python 3.12+
# see https://github.com/python/cpython/issues/105524
def __contains__(self, key):
return standardize_formula(key) in self.data

def __delitem__(self, key):
super().__delitem__(standardize_formula(key))
2 changes: 2 additions & 0 deletions tests/test_activity.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
cases, the output is also tested against a well-established model published
by USGS(PHREEQC)
"""
import platform

import numpy as np
import pytest
Expand Down Expand Up @@ -280,6 +281,7 @@ def test_activity_crc_rbcl():
assert np.isclose(result, expected, rtol=0.05)


@pytest.mark.skipif(platform.machine() == "arm64" and platform.system() == "Darwin", reason="arm64 not supported")
def test_activity_crc_MgCl2():
"""
calculate the activity coefficient of MgCl2 at each concentration and compare
Expand Down
3 changes: 3 additions & 0 deletions tests/test_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
Tests of pyEQL.functions module
"""
import platform

import numpy as np
import pytest

Expand Down Expand Up @@ -39,6 +41,7 @@ def s2_i():
return Solution({"Na+": "1 mol/L", "Cl-": "1 mol/L"}, volume="10 L", engine="ideal")


@pytest.mark.skipif(platform.machine() == "arm64" and platform.system() == "Darwin", reason="arm64 not supported")
def test_mixing_functions(s1, s2, s1_p, s2_p, s1_i, s2_i):
# mixing energy and entropy of any solution with itself should be zero
assert np.isclose(gibbs_mix(s1, s1).magnitude, 0)
Expand Down
4 changes: 4 additions & 0 deletions tests/test_phreeqc.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,17 @@
"""

import logging
import platform

import numpy as np
import pytest

from pyEQL import Solution
from pyEQL.engines import PhreeqcEOS

if platform.machine().startswith("arm64") and platform.system().startswith("Darwin"):
pytest.skip("skipping PHREEQC tests because arm64 is not supported", allow_module_level=True)


@pytest.fixture()
def s1():
Expand Down
3 changes: 3 additions & 0 deletions tests/test_salt_matching.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
This file contains tests for the salt-matching algorithm used by pyEQL in
salt_ion_match.py
"""
import platform

import numpy as np
import pytest

import pyEQL
from pyEQL.salt_ion_match import Salt
Expand Down Expand Up @@ -97,6 +99,7 @@ def test_single_ion():
assert s1.get_salt().nu_anion == 3


@pytest.mark.skipif(platform.machine() == "arm64" and platform.system() == "Darwin", reason="arm64 not supported")
def test_salt_with_equilibration():
"""
test matching a solution containing a salt, before and after equilibration.
Expand Down
5 changes: 5 additions & 0 deletions tests/test_solution.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import copy
import os
import platform
from pathlib import Path

import numpy as np
Expand Down Expand Up @@ -366,6 +367,8 @@ def test_components_by_element(s1, s2):
"Na(1.0)": ["Na[+1]"],
"Cl(-1.0)": ["Cl[-1]"],
}
if platform.machine() == "arm64" and platform.system() == "Darwin":
pytest.skip(reason="arm64 not supported")
s2.equilibrate()
assert s2.get_components_by_element() == {
"H(1.0)": ["H2O(aq)", "OH[-1]", "H[+1]", "HCl(aq)", "NaOH(aq)", "HClO(aq)", "HClO2(aq)"],
Expand Down Expand Up @@ -401,6 +404,7 @@ def test_get_total_amount(s2):
assert np.isclose(sox.get_total_amount("Fe", "mol").magnitude, 0.05)


@pytest.mark.skipif(platform.machine() == "arm64" and platform.system() == "Darwin", reason="arm64 not supported")
def test_equilibrate(s1, s2, s5_pH):
assert "H2(aq)" not in s1.components
orig_pH = s1.pH
Expand Down Expand Up @@ -505,6 +509,7 @@ def test_conductivity(s1, s2):
assert np.isclose(s_kcl.conductivity.magnitude, 10.862, atol=0.45)


@pytest.mark.skipif(platform.machine() == "arm64" and platform.system() == "Darwin", reason="arm64 not supported")
def test_arithmetic_and_copy(s2, s6):
s6_scale = copy.deepcopy(s6)
s6_scale *= 1.5
Expand Down

0 comments on commit 28ce5fc

Please sign in to comment.