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

Address GC with JPype ≥1.5.0 #504

Merged
merged 14 commits into from
Jan 9, 2024
Merged
20 changes: 9 additions & 11 deletions .github/workflows/pytest.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ jobs:
- "3.8" # Earliest version supported by ixmp
- "3.9"
- "3.10"
- "3.11" # Latest supported by ixmp
# - "3.12" # Pending JPype support; see iiasa/ixmp#501
- "3.11"
- "3.12" # Latest supported by ixmp

# commented: force a specific version of pandas, for e.g. pre-release
# testing
Expand Down Expand Up @@ -92,10 +92,6 @@ jobs:
run: echo "RETICULATE_PYTHON=$pythonLocation" >> $GITHUB_ENV
shell: bash

- name: Work around https://bugs.launchpad.net/lxml/+bug/2035206
if: matrix.python-version == '3.8' && matrix.os == 'macos-latest'
run: pip install "lxml == 4.9.2"

- name: Install Python package and dependencies
# [docs] contains [tests], which contains [report,tutorial]
run: |
Expand All @@ -104,10 +100,6 @@ jobs:
# commented: use with "pandas-version" in the matrix, above
# pip install --upgrade pandas${{ matrix.pandas-version }}

- name: TEMPORARY Work around iiasa/ixmp#463
if: matrix.python-version != '3.11'
run: pip install "JPype1 != 1.4.1"

- name: Install R dependencies and tutorial requirements
run: |
install.packages(c("remotes", "Rcpp"))
Expand All @@ -125,7 +117,13 @@ jobs:
shell: Rscript {0}

- name: Run test suite using pytest
run: pytest ixmp -m "not performance" --verbose -rA --cov-report=xml --color=yes
run: |
pytest ixmp \
-m "not performance" \
--color=yes -rA --verbose \
--cov-report=xml \
--numprocesses=auto
shell: bash

- name: Upload test coverage to Codecov.io
uses: codecov/codecov-action@v3
Expand Down
6 changes: 5 additions & 1 deletion RELEASE_NOTES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ Code that imports from the old locations will continue to work, but will raise :
All changes
-----------

- Support for Python 3.7 is dropped (:pull:`492`).
- :mod:`ixmp` is tested and compatible with `Python 3.12 <https://www.python.org/downloads/release/python-3120/>`__ (:pull:`504`).
- Support for Python 3.7 is dropped (:pull:`492`), as it has reached end-of-life.
- Rename :mod:`ixmp.report` and :mod:`ixmp.util` (:pull:`500`).
- New reporting operators :func:`.from_url`, :func:`.get_ts`, and :func:`.remove_ts` (:pull:`500`).
- New CLI command :program:`ixmp platform copy` and :doc:`CLI documentation <cli>` (:pull:`500`).
Expand All @@ -28,6 +29,9 @@ All changes
- When a :class:`.GAMSModel` is solved with an LP status of 5 (optimal, but with infeasibilities after unscaling), :class:`.JDBCBackend` would attempt to read the output GDX file and fail, leading to an uninformative error message (:issue:`98`).
Now :class:`.ModelError` is raised describing the situation.
- Improved type hinting for static typing of code that uses :mod:`ixmp` (:issue:`465`, :pull:`500`).
- :mod:`ixmp` requires JPype1 1.4.0 or earlier, for Python 3.10 and earlier (:pull:`504`).
With JPype1 1.4.1 and later, memory management in :class:`.CachingBackend` may not function as intended (:issue:`463`), which could lead to high memory use where many, large :class:`.Scenario` objects are created and used in a single Python program.
(For Python 3.11 and later, any version of JPype1 from the prior minimum (1.2.1) to the latest is supported.)

.. _v3.7.0:

Expand Down
19 changes: 13 additions & 6 deletions ixmp/backend/jdbc.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,7 @@ def _raise_jexception(exc, msg="unhandled Java exception: "):
"""Convert Java/JPype exceptions to ordinary Python RuntimeError."""
# Try to re-raise as a ValueError for bad model or scenario name
arg = exc.args[0] if isinstance(exc.args[0], str) else ""
match = re.search(r"getting '([^']*)' in table '([^']*)'", arg)
if match:
if match := re.search(r"getting '([^']*)' in table '([^']*)'", arg):
param = match.group(2).lower()
if param in {"model", "scenario"}:
raise ValueError(f"{param}={repr(match.group(1))}") from None
Expand Down Expand Up @@ -301,7 +300,7 @@ def __init__(self, jvmargs=None, **kwargs):
f"{e}\nCheck that dependencies of ixmp.jar are "
f"included in {Path(__file__).parents[2] / 'lib'}"
)
except jpype.JException as e: # pragma: no cover
except java.Exception as e: # pragma: no cover
# Handle Java exceptions
jclass = e.__class__.__name__
if jclass.endswith("HikariPool.PoolInitializationException"):
Expand Down Expand Up @@ -708,10 +707,18 @@ def get(self, ts):
# either getTimeSeries or getScenario
method = getattr(self.jobj, "get" + ts.__class__.__name__)

# Re-raise as a ValueError for bad model or scenario name, or other
with _handle_jexception():
# Re-raise as a ValueError for bad model or scenario name, or other with
# with _handle_jexception():
try:
# Either the 2- or 3- argument form, depending on args
jobj = method(*args)
except SystemError:
# JPype 1.5.0 with Python 3.12: "<built-in method __subclasscheck__ of
# _jpype._JClass object at …> returned a result with an exception set"
# At least transmute to a ValueError
raise ValueError("model, scenario, or version not found")
except BaseException as e:
_raise_jexception(e)

self._index_and_set_attrs(jobj, ts)

Expand Down Expand Up @@ -938,7 +945,7 @@ def init_item(self, s, type, name, idx_sets, idx_names):
# by Backend, so don't return here
try:
func(name, idx_sets, idx_names)
except jpype.JException as e:
except java.Exception as e:
if "already exists" in e.args[0]:
raise ValueError(f"{repr(name)} already exists")
else:
Expand Down
2 changes: 1 addition & 1 deletion ixmp/core/scenario.py
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ def add_par(
# Multiple values
values = value

data = pd.DataFrame(zip(keys, values), columns=["key", "value"])
data = pd.DataFrame(zip_longest(keys, values), columns=["key", "value"])
if data.isna().any(axis=None):
raise ValueError("Length mismatch between keys and values")

Expand Down
11 changes: 10 additions & 1 deletion ixmp/tests/backend/test_base.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import sys
from pathlib import Path

import pytest
Expand Down Expand Up @@ -69,7 +70,9 @@ class BE2(Backend):
def test_class():
# An incomplete Backend subclass can't be instantiated
with pytest.raises(
TypeError, match="Can't instantiate abstract class BE1 with abstract methods"
TypeError,
match="Can't instantiate abstract class BE1 with(out an implementation for)? "
"abstract methods",
):
BE1()

Expand Down Expand Up @@ -145,6 +148,12 @@ def test_del_ts(self, test_mp):
# Cache size has increased
assert cache_size_pre + 1 == len(backend._cache)

# JPype ≥ 1.4.1 with Python ≤ 3.10 produces danging traceback/frame references
# to `s` that prevent it being GC'd at "del s" below. See
# https://github.com/iiasa/ixmp/issues/463 and test_jdbc.test_del_ts
if sys.version_info.minor <= 10:
s.__del__() # Force deletion of cached objects associated with `s`

# Delete the object; associated cache is freed
del s

Expand Down
7 changes: 7 additions & 0 deletions ixmp/tests/backend/test_jdbc.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import logging
import os
import platform
import sys
from sys import getrefcount
from typing import Tuple

Expand Down Expand Up @@ -370,6 +371,12 @@ def test_verbose_exception(test_mp, exception_verbose_true):
assert "at.ac.iiasa.ixmp.Platform.getScenario" in exc_msg


@pytest.mark.xfail(
condition=sys.version_info.minor <= 10,
raises=AssertionError,
# See also test_base.TestCachingBackend.test_del_ts
reason="https://github.com/iiasa/ixmp/issues/463",
)
def test_del_ts():
mp = ixmp.Platform(
backend="jdbc",
Expand Down
12 changes: 9 additions & 3 deletions ixmp/tests/core/test_scenario.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import re
import sys

import numpy.testing as npt
import pandas as pd
Expand Down Expand Up @@ -93,10 +94,15 @@ def test_from_url(self, mp, caplog):
with pytest.raises(Exception, match=expected):
scen, mp = ixmp.Scenario.from_url(url + "#10000", errors="raise")

# Giving an invalid scenario with errors='warn' raises an exception
# Giving an invalid scenario with errors='warn' causes a message to be logged
msg = (
"ValueError: scenario='Hitchhikerfoo'\n"
f"when loading Scenario from url: {repr(url + 'foo')}"
"ValueError: "
+ (
"scenario='Hitchhikerfoo'"
if sys.version_info.minor != 12
else "model, scenario, or version not found"
)
+ f"\nwhen loading Scenario from url: {repr(url + 'foo')}"
)
with assert_logs(caplog, msg):
scen, mp = ixmp.Scenario.from_url(url + "foo")
Expand Down
8 changes: 7 additions & 1 deletion ixmp/tests/test_cli.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import re
import sys
from pathlib import Path

import pandas as pd
Expand Down Expand Up @@ -403,7 +404,12 @@ def test_solve(ixmp_cli, test_mp):
]
result = ixmp_cli.invoke(cmd)
assert result.exit_code == 1, result.output
assert "Error: model='non-existing'" in result.output
exp = (
"='non-existing'"
if sys.version_info.minor != 12
else ", scenario, or version not found"
)
assert f"Error: model{exp}" in result.output

result = ixmp_cli.invoke([f"--url=ixmp://{test_mp.name}/foo/bar", "solve"])
assert UsageError.exit_code == result.exit_code, result.output
Expand Down
4 changes: 3 additions & 1 deletion ixmp/tests/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ class M1(Model):
pass

with pytest.raises(
TypeError, match="Can't instantiate abstract class M1 " "with abstract methods"
TypeError,
match="Can't instantiate abstract class M1 with(out an implementation for)? "
"abstract methods",
):
M1()

Expand Down
10 changes: 5 additions & 5 deletions ixmp/tests/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,17 +92,17 @@ def test_diff_data(test_mp):
# Expected results
exp_b = pd.DataFrame(
[
["chicago", 300.0, "cases", np.NaN, None, "left_only"],
["new-york", np.NaN, None, 325.0, "cases", "right_only"],
["chicago", 300.0, "cases", np.nan, np.nan, "left_only"],
["new-york", np.nan, np.nan, 325.0, "cases", "right_only"],
["topeka", 275.0, "cases", 275.0, "cases", "both"],
],
columns="j value_a unit_a value_b unit_b _merge".split(),
)
exp_d = pd.DataFrame(
[
["san-diego", "chicago", np.NaN, None, 1.8, "km", "right_only"],
["san-diego", "new-york", np.NaN, None, 2.5, "km", "right_only"],
["san-diego", "topeka", np.NaN, None, 1.4, "km", "right_only"],
["san-diego", "chicago", np.nan, np.nan, 1.8, "km", "right_only"],
["san-diego", "new-york", np.nan, np.nan, 2.5, "km", "right_only"],
["san-diego", "topeka", np.nan, np.nan, 1.4, "km", "right_only"],
["seattle", "chicago", 1.7, "km", 123.4, "km", "both"],
["seattle", "new-york", 2.5, "km", 123.4, "km", "both"],
["seattle", "topeka", 1.8, "km", 123.4, "km", "both"],
Expand Down
8 changes: 3 additions & 5 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ classifiers = [
"Programming Language :: Python :: 3.9",
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
"Programming Language :: Python :: 3.12",
"Programming Language :: R",
"Topic :: Scientific/Engineering",
"Topic :: Scientific/Engineering :: Information Analysis",
Expand All @@ -33,6 +34,7 @@ dependencies = [
"click",
"genno >= 1.16",
"JPype1 >= 1.2.1",
"JPype1 <= 1.4.0; python_version < '3.11'",
"openpyxl",
"pandas >= 1.2",
"pint",
Expand Down Expand Up @@ -65,6 +67,7 @@ tests = [
"pytest-benchmark",
"pytest-cov",
"pytest-rerunfailures",
"pytest-xdist",
]

[project.scripts]
Expand All @@ -77,11 +80,6 @@ exclude_also = [
]
omit = ["ixmp/util/sphinx_linkcode_github.py"]

[tool.isort]
profile = "black"

[tool.mypy]

[[tool.mypy.overrides]]
# Packages/modules for which no type hints are available.
module = [
Expand Down