Merge branch 'develop' of github.com:kratman/PyBaMM into feat/removeIDAKLU

kratman · kratman · commit c89e190f1d91 · 2024-11-13T17:33:33.000-05:00
diff --git a/.github/workflows/lychee_url_checker.yml b/.github/workflows/lychee_url_checker.yml
@@ -28,7 +28,7 @@ jobs:
 
       # use stable version for now to avoid breaking changes
       - name: Lychee URL checker
-        uses: lycheeverse/lychee-action@v2.0.2
+        uses: lycheeverse/lychee-action@v2.1.0
         with:
           # arguments with file types to check
           args: >-
diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml
@@ -68,6 +68,6 @@ jobs:
       # Upload the results to GitHub's code scanning dashboard (optional).
       # Commenting out will disable upload of results to your repo's Code Scanning dashboard
       - name: "Upload to code-scanning"
-        uses: github/codeql-action/upload-sarif@662472033e021d55d94146f66f6058822b0b39fd # v3.27.0
+        uses: github/codeql-action/upload-sarif@4f3212b61783c3c68e8309a0f18a699764811cda # v3.27.1
         with:
           sarif_file: results.sarif
diff --git a/.gitignore b/.gitignore
@@ -46,7 +46,6 @@ input/*
 
 # simulation outputs
 out/
-config.py
 matplotlibrc
 *.pickle
 *.sav
diff --git a/.lycheeignore b/.lycheeignore
@@ -15,3 +15,6 @@ file:///home/runner/work/PyBaMM/PyBaMM/docs/source/user_guide/fundamentals/pybam
 
 # Errors in docs/source/user_guide/index.md
 file:///home/runner/work/PyBaMM/PyBaMM/docs/source/user_guide/api_docs
+
+# Telemetry
+https://us.i.posthog.com
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -4,7 +4,7 @@ ci:
 
 repos:
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: "v0.7.2"
+    rev: "v0.7.3"
     hooks:
       - id: ruff
         args: [--fix, --show-fixes]
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,15 +2,16 @@
 
 ## Features
 
+- Added `CoupledVariable` which provides a placeholder variable whose equation can be elsewhere in the model. ([#4556](https://github.com/pybamm-team/PyBaMM/pull/4556))
 - Adds support to `pybamm.Experiment` for the `output_variables` option in the `IDAKLUSolver`. ([#4534](https://github.com/pybamm-team/PyBaMM/pull/4534))
 - Adds an option "voltage as a state" that can be "false" (default) or "true". If "true" adds an explicit algebraic equation for the voltage. ([#4507](https://github.com/pybamm-team/PyBaMM/pull/4507))
 - Improved `QuickPlot` accuracy for simulations with Hermite interpolation. ([#4483](https://github.com/pybamm-team/PyBaMM/pull/4483))
 - Added Hermite interpolation to the (`IDAKLUSolver`) that improves the accuracy and performance of post-processing variables. ([#4464](https://github.com/pybamm-team/PyBaMM/pull/4464))
+- Added basic telemetry to record which functions are being run. See [Telemetry section in the User Guide](https://docs.pybamm.org/en/latest/source/user_guide/index.html#telemetry) for more information. ([#4441](https://github.com/pybamm-team/PyBaMM/pull/4441))
 - Added `BasicDFN` model for sodium-ion batteries ([#4451](https://github.com/pybamm-team/PyBaMM/pull/4451))
 - Added sensitivity calculation support for `pybamm.Simulation` and `pybamm.Experiment` ([#4415](https://github.com/pybamm-team/PyBaMM/pull/4415))
 - Added OpenMP parallelization to IDAKLU solver for lists of input parameters ([#4449](https://github.com/pybamm-team/PyBaMM/pull/4449))
-- Added phase-dependent particle options to LAM
-  ([#4369](https://github.com/pybamm-team/PyBaMM/pull/4369))
+- Added phase-dependent particle options to LAM ([#4369](https://github.com/pybamm-team/PyBaMM/pull/4369))
 - Added a lithium ion equivalent circuit model with split open circuit voltages for each electrode (`SplitOCVR`). ([#4330](https://github.com/pybamm-team/PyBaMM/pull/4330))
 - Added the `pybamm.DiscreteTimeSum` expression node to sum an expression over a sequence of data times, and accompanying `pybamm.DiscreteTimeData` class to store the data times and values ([#4501](https://github.com/pybamm-team/PyBaMM/pull/4501))
 
@@ -21,6 +22,8 @@
 - Removed the `start_step_offset` setting and disabled minimum `dt` warnings for drive cycles with the (`IDAKLUSolver`). ([#4416](https://github.com/pybamm-team/PyBaMM/pull/4416))
 
 ## Bug Fixes
+- Added error for binary operators on two concatenations with different numbers of children. Previously, the extra children were dropped. Also fixed bug where Q_rxn was dropped from the total heating term in half-cell models. ([#4562](https://github.com/pybamm-team/PyBaMM/pull/4562))
+- Fixed bug where Q_rxn was set to 0 for the negative electrode in half-cell models. ([#4557](https://github.com/pybamm-team/PyBaMM/pull/4557))
 - Fixed bug in post-processing solutions with infeasible experiments using the (`IDAKLUSolver`). ([#4541](https://github.com/pybamm-team/PyBaMM/pull/4541))
 - Disabled IREE on MacOS due to compatibility issues and added the CasADI
   path to the environment to resolve issues on MacOS and Linux. Windows
diff --git a/conftest.py b/conftest.py
@@ -51,3 +51,8 @@ def set_random_seed():
 @pytest.fixture(autouse=True)
 def set_debug_value():
     pybamm.settings.debug_mode = True
+
+
+@pytest.fixture(autouse=True)
+def disable_telemetry():
+    pybamm.telemetry.disable()
diff --git a/docs/source/user_guide/index.md b/docs/source/user_guide/index.md
@@ -73,3 +73,12 @@ glob:
 ../examples/notebooks/creating_models/5-half-cell-model.ipynb
 ../examples/notebooks/creating_models/6-a-simple-SEI-model.ipynb
 ```
+
+# Telemetry
+
+PyBaMM optionally collects anonymous usage data to help improve the library. This telemetry is opt-in and can be easily disabled. Here's what you need to know:
+
+- **What is collected**: Basic usage information like PyBaMM version, Python version, and which functions are run.
+- **Why**: To understand how PyBaMM is used and prioritize development efforts.
+- **Opt-out**: To disable telemetry, set the environment variable `PYBAMM_DISABLE_TELEMETRY=true` (or any value other than `false`) or use `pybamm.telemetry.disable()` in your code.
+- **Privacy**: No personal information (name, email, etc) or sensitive information (parameter values, simulation results, etc) is ever collected.
diff --git a/docs/source/user_guide/installation/index.rst b/docs/source/user_guide/installation/index.rst
@@ -71,6 +71,7 @@ Package                                                             Minimum supp
 `typing-extensions <https://pypi.org/project/typing-extensions/>`__ 4.10.0
 `pandas <https://pypi.org/project/pandas/>`__                       1.5.0
 `pooch <https://www.fatiando.org/pooch/>`__                         1.8.1
+`posthog <https://posthog.com/>`__                                  3.6.5
 =================================================================== ==========================
 
 .. _install.optional_dependencies:
diff --git a/pyproject.toml b/pyproject.toml
@@ -29,16 +29,17 @@ classifiers = [
     "Topic :: Scientific/Engineering",
 ]
 dependencies = [
+    "pybammsolvers",
     "numpy>=1.23.5,<2.0.0",
     "scipy>=1.11.4",
-    "casadi>=3.6.6",
+    "casadi>=3.6.7",
     "xarray>=2022.6.0",
     "anytree>=2.8.0",
     "sympy>=1.12",
     "typing-extensions>=4.10.0",
     "pandas>=1.5.0",
     "pooch>=1.8.1",
-    "pybammsolvers",
+    "posthog",
 ]
 
 [project.urls]
diff --git a/src/pybamm/__init__.py b/src/pybamm/__init__.py
@@ -21,6 +21,7 @@
 from .logger import logger, set_logging_level, get_new_logger
 from .settings import settings
 from .citations import Citations, citations, print_citations
+from . import config
 
 # Classes for the Expression Tree
 from .expression_tree.symbol import *
@@ -194,8 +195,8 @@
 # Batch Study
 from .batch_study import BatchStudy
 
-# Callbacks
-from . import callbacks
+# Callbacks, telemetry, config
+from . import callbacks, telemetry, config
 
 # Pybamm Data manager using pooch
 from .pybamm_data import DataLoader
@@ -204,12 +205,14 @@
 import os
 import pathlib
 import sysconfig
-os.environ["CASADIPATH"] = str(pathlib.Path(sysconfig.get_path('purelib')) / 'casadi')
+
+os.environ["CASADIPATH"] = str(pathlib.Path(sysconfig.get_path("purelib")) / "casadi")
 
 __all__ = [
     "batch_study",
     "callbacks",
     "citations",
+    "config",
     "discretisations",
     "doc_utils",
     "experiment",
@@ -225,8 +228,11 @@
     "simulation",
     "solvers",
     "spatial_methods",
+    "telemetry",
     "type_definitions",
     "util",
     "version",
     "pybamm_data",
 ]
+
+config.generate()
diff --git a/src/pybamm/config.py b/src/pybamm/config.py
@@ -0,0 +1,163 @@
+import uuid
+import os
+import platformdirs
+from pathlib import Path
+import pybamm
+import sys
+import threading
+import time
+
+
+def is_running_tests():  # pragma: no cover
+    """
+    Detect if the code is being run as part of a test suite or building docs with Sphinx.
+
+    Returns:
+        bool: True if running tests or building docs, False otherwise.
+    """
+    # Check if pytest or unittest is running
+    if any(
+        test_module in sys.modules for test_module in ["pytest", "unittest", "nose"]
+    ):
+        return True
+
+    # Check for GitHub Actions environment variable
+    if "GITHUB_ACTIONS" in os.environ:
+        return True
+
+    # Check for other common CI environment variables
+    ci_env_vars = ["CI", "TRAVIS", "CIRCLECI", "JENKINS_URL", "GITLAB_CI"]
+    if any(var in os.environ for var in ci_env_vars):
+        return True
+
+    # Check for common test runner names in command-line arguments
+    test_runners = ["pytest", "unittest", "nose", "trial", "nox", "tox"]
+    if any(runner in arg.lower() for arg in sys.argv for runner in test_runners):
+        return True
+
+    # Check if building docs with Sphinx
+    if any(mod == "sphinx" or mod.startswith("sphinx.") for mod in sys.modules):
+        print(
+            f"Found Sphinx module: {[mod for mod in sys.modules if mod.startswith('sphinx')]}"
+        )
+        return True
+
+    return False
+
+
+def ask_user_opt_in(timeout=10):
+    """
+    Ask the user if they want to opt in to telemetry.
+
+    Parameters
+    ----------
+    timeout : float, optional
+        The timeout for the user to respond to the prompt. Default is 10 seconds.
+
+    Returns
+    -------
+    bool
+        True if the user opts in, False otherwise.
+    """
+    print(
+        "PyBaMM can collect usage data and send it to the PyBaMM team to "
+        "help us improve the software.\n"
+        "We do not collect any sensitive information such as models, parameters, "
+        "or simulation results - only information on which parts of the code are "
+        "being used and how frequently.\n"
+        "This is entirely optional and does not impact the functionality of PyBaMM.\n"
+        "For more information, see https://docs.pybamm.org/en/latest/source/user_guide/index.html#telemetry"
+    )
+
+    def get_input():  # pragma: no cover
+        try:
+            user_input = (
+                input("Do you want to enable telemetry? (Y/n): ").strip().lower()
+            )
+            answer.append(user_input)
+        except Exception:
+            # Handle any input errors
+            pass
+
+    time_start = time.time()
+
+    while True:
+        if time.time() - time_start > timeout:
+            print("\nTimeout reached. Defaulting to not enabling telemetry.")
+            return False
+
+        answer = []
+        # Create and start input thread
+        input_thread = threading.Thread(target=get_input)
+        input_thread.daemon = True
+        input_thread.start()
+
+        # Wait for either timeout or input
+        input_thread.join(timeout)
+
+        if answer:
+            if answer[0] in ["yes", "y", ""]:
+                print("\nTelemetry enabled.\n")
+                return True
+            elif answer[0] in ["no", "n"]:
+                print("\nTelemetry disabled.\n")
+                return False
+            else:
+                print("\nInvalid input. Please enter 'yes/y' for yes or 'no/n' for no.")
+        else:
+            print("\nTimeout reached. Defaulting to not enabling telemetry.")
+            return False
+
+
+def generate():
+    if is_running_tests():
+        return
+
+    # Check if the config file already exists
+    if read() is not None:
+        return
+
+    # Ask the user if they want to opt in to telemetry
+    opt_in = ask_user_opt_in()
+    config_file = Path(platformdirs.user_config_dir("pybamm")) / "config.yml"
+    write_uuid_to_file(config_file, opt_in)
+
+    if opt_in:
+        pybamm.telemetry.capture("user-opted-in")
+
+
+def read():
+    config_file = Path(platformdirs.user_config_dir("pybamm")) / "config.yml"
+    return read_uuid_from_file(config_file)
+
+
+def write_uuid_to_file(config_file, opt_in):
+    # Create the directory if it doesn't exist
+    config_file.parent.mkdir(parents=True, exist_ok=True)
+
+    # Write the UUID to the config file in YAML format
+    with open(config_file, "w") as f:
+        f.write("pybamm:\n")
+        f.write(f"  enable_telemetry: {opt_in}\n")
+        if opt_in:
+            unique_id = uuid.uuid4()
+            f.write(f"  uuid: {unique_id}\n")
+
+
+def read_uuid_from_file(config_file):
+    # Check if the config file exists
+    if not config_file.exists():
+        return None
+
+    # Read the UUID from the config file
+    with open(config_file) as f:
+        content = f.read().strip()
+
+    # Extract the UUID using YAML parsing
+    try:
+        import yaml
+
+        config = yaml.safe_load(content)
+        return config["pybamm"]
+    except (yaml.YAMLError, ValueError):
+        return None
diff --git a/src/pybamm/expression_tree/binary_operators.py b/src/pybamm/expression_tree/binary_operators.py
@@ -856,12 +856,17 @@ def _simplified_binary_broadcast_concatenation(
         elif isinstance(right, pybamm.Concatenation) and not isinstance(
             right, pybamm.ConcatenationVariable
         ):
-            return left.create_copy(
-                [
-                    operator(left_child, right_child)
-                    for left_child, right_child in zip(left.orphans, right.orphans)
-                ]
-            )
+            if len(left.orphans) == len(right.orphans):
+                return left.create_copy(
+                    [
+                        operator(left_child, right_child)
+                        for left_child, right_child in zip(left.orphans, right.orphans)
+                    ]
+                )
+            else:
+                raise AssertionError(
+                    "Concatenations must have the same number of children"
+                )
     if isinstance(right, pybamm.Concatenation) and not isinstance(
         right, pybamm.ConcatenationVariable
     ):
diff --git a/src/pybamm/models/full_battery_models/base_battery_model.py b/src/pybamm/models/full_battery_models/base_battery_model.py
@@ -989,7 +989,6 @@ def options(self, extra_options):
             raise pybamm.OptionError(
                 f"must use surface formulation to solve {self!s} with hydrolysis"
             )
-
         self._options = options
 
     def set_standard_output_variables(self):
diff --git a/src/pybamm/models/submodels/thermal/base_thermal.py b/src/pybamm/models/submodels/thermal/base_thermal.py
@@ -88,6 +88,12 @@ def _get_standard_coupled_variables(self, variables):
         # TODO: change full stefan-maxwell conductivity so that i_e is always
         # a Concatenation
         i_e = variables["Electrolyte current density [A.m-2]"]
+        # Special case for half cell -- i_e has to be a concatenation for this to work due to a mismatch with Q_ohm, so we make a new i_e which is a concatenation.
+        if (not isinstance(i_e, pybamm.Concatenation)) and (
+            self.options.electrode_types["negative"] == "planar"
+        ):
+            i_e = self._get_i_e_for_half_cell_thermal(variables)
+
         phi_e = variables["Electrolyte potential [V]"]
         if isinstance(i_e, pybamm.Concatenation):
             # compute by domain if possible
@@ -411,3 +417,23 @@ def _yz_average(self, var):
             return pybamm.z_average(var)
         elif self.options["dimensionality"] == 2:
             return pybamm.yz_average(var)
+
+    def _get_i_e_for_half_cell_thermal(self, variables):
+        c_e_s = variables["Separator electrolyte concentration [mol.m-3]"]
+        c_e_p = variables["Positive electrolyte concentration [mol.m-3]"]
+        grad_phi_e_s = variables["Gradient of separator electrolyte potential [V.m-1]"]
+        grad_phi_e_p = variables["Gradient of positive electrolyte potential [V.m-1]"]
+        grad_c_e_s = pybamm.grad(c_e_s)
+        grad_c_e_p = pybamm.grad(c_e_p)
+        T_s = variables["Separator temperature [K]"]
+        T_p = variables["Positive electrode temperature [K]"]
+        tor_s = variables["Separator electrolyte transport efficiency"]
+        tor_p = variables["Positive electrolyte transport efficiency"]
+        i_e_s = (self.param.kappa_e(c_e_s, T_s) * tor_s) * (
+            self.param.chiRT_over_Fc(c_e_s, T_s) * grad_c_e_s - grad_phi_e_s
+        )
+        i_e_p = (self.param.kappa_e(c_e_p, T_p) * tor_p) * (
+            self.param.chiRT_over_Fc(c_e_p, T_p) * grad_c_e_p - grad_phi_e_p
+        )
+        i_e = pybamm.concatenation(i_e_s, i_e_p)
+        return i_e
diff --git a/src/pybamm/parameters/parameter_values.py b/src/pybamm/parameters/parameter_values.py
diff --git a/src/pybamm/simulation.py b/src/pybamm/simulation.py
diff --git a/src/pybamm/telemetry.py b/src/pybamm/telemetry.py
diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py
diff --git a/tests/unit/test_expression_tree/test_concatenations.py b/tests/unit/test_expression_tree/test_concatenations.py
