Repository: stan-dev/cmdstanpy
Pull request #700 (2023-09-20 15:12:09)

Move compilation code, deprecate constructing a model without ever compiling
The pre-2.0 part of #695 

#### Submission Checklist

- [x] Run unit tests
- [x] Declare copyright holder and open-source license: see below

#### Summary

This moves the compilation code outside the model class and issues deprecation warnings for cases we'd like to disallow in 2.0, namely the use of a CmdStanModel without an executable at all.

I think if we get this in, I can update #691, and then we can release 1.2.0 

#### Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company):

Simons Foundation.

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)




Patch:
diff --git a/cmdstanpy/__init__.py b/cmdstanpy/__init__.py
index 93c2107..815f1ab 100644
--- a/cmdstanpy/__init__.py
+++ b/cmdstanpy/__init__.py
@@ -22,6 +22,7 @@ atexit.register(_cleanup_tmpdir)
 
 
 from ._version import __version__  # noqa
+from .compilation import compile_stan_file
 from .install_cmdstan import rebuild_cmdstan
 from .model import CmdStanModel
 from .stanfit import (
@@ -31,7 +32,6 @@ from .stanfit import (
     CmdStanMLE,
     CmdStanPathfinder,
     CmdStanVB,
-    InferenceMetadata,
     from_csv,
 )
 from .utils import (
@@ -49,6 +49,7 @@ __all__ = [
     'cmdstan_path',
     'set_make_env',
     'install_cmdstan',
+    'compile_stan_file',
     'CmdStanMCMC',
     'CmdStanMLE',
     'CmdStanGQ',
@@ -56,7 +57,6 @@ __all__ = [
     'CmdStanLaplace',
     'CmdStanPathfinder',
     'CmdStanModel',
-    'InferenceMetadata',
     'from_csv',
     'write_stan_json',
     'show_versions',
diff --git a/cmdstanpy/compiler_opts.py b/cmdstanpy/compilation.py
similarity index 67%
rename from cmdstanpy/compiler_opts.py
rename to cmdstanpy/compilation.py
index 3eb9950..a95e79e 100644
--- a/cmdstanpy/compiler_opts.py
+++ b/cmdstanpy/compilation.py
@@ -2,12 +2,20 @@
 Makefile options for stanc and C++ compilers
 """
 
+import io
+import json
 import os
+import platform
+import shutil
+import subprocess
 from copy import copy
 from pathlib import Path
 from typing import Any, Dict, Iterable, List, Optional, Union
 
 from cmdstanpy.utils import get_logger
+from cmdstanpy.utils.cmdstan import EXTENSION, cmdstan_path
+from cmdstanpy.utils.command import do_command
+from cmdstanpy.utils.filesystem import SanitizedOrTmpFilePath
 
 STANC_OPTS = [
     'O',
@@ -258,8 +266,9 @@ class CompilerOptions:
             else:
                 for key, val in new_opts.stanc_options.items():
                     if key == 'include-paths':
-                        if isinstance(val, Iterable) \
-                                and not isinstance(val, str):
+                        if isinstance(val, Iterable) and not isinstance(
+                            val, str
+                        ):
                             for path in val:
                                 self.add_include_path(str(path))
                         else:
@@ -322,3 +331,146 @@ class CompilerOptions:
             for key, val in self._cpp_options.items():
                 opts.append(f'{key}={val}')
         return opts
+
+
+def src_info(
+    stan_file: str, compiler_options: CompilerOptions
+) -> Dict[str, Any]:
+    """
+    Get source info for Stan program file.
+
+    This function is used in the implementation of
+    :meth:`CmdStanModel.src_info`, and should not be called directly.
+    """
+    cmd = (
+        [os.path.join(cmdstan_path(), 'bin', 'stanc' + EXTENSION)]
+        # handle include-paths, allow-undefined etc
+        + compiler_options.compose_stanc(None)
+        + ['--info', str(stan_file)]
+    )
+    proc = subprocess.run(cmd, capture_output=True, text=True, check=False)
+    if proc.returncode:
+        raise ValueError(
+            f"Failed to get source info for Stan model "
+            f"'{stan_file}'. Console:\n{proc.stderr}"
+        )
+    result: Dict[str, Any] = json.loads(proc.stdout)
+    return result
+
+
+def compile_stan_file(
+    src: Union[str, Path],
+    force: bool = False,
+    stanc_options: Optional[Dict[str, Any]] = None,
+    cpp_options: Optional[Dict[str, Any]] = None,
+    user_header: OptionalPath = None,
+) -> str:
+    """
+    Compile the given Stan program file.  Translates the Stan code to
+    C++, then calls the C++ compiler.
+
+    By default, this function compares the timestamps on the source and
+    executable files; if the executable is newer than the source file, it
+    will not recompile the file, unless argument ``force`` is ``True``
+    or unless the compiler options have been changed.
+
+    :param src: Path to Stan program file.
+
+    :param force: When ``True``, always compile, even if the executable file
+        is newer than the source file.  Used for Stan models which have
+        ``#include`` directives in order to force recompilation when changes
+        are made to the included files.
+
+    :param stanc_options: Options for stanc compiler.
+    :param cpp_options: Options for C++ compiler.
+    :param user_header: A path to a header file to include during C++
+        compilation.
+    """
+
+    src = Path(src).resolve()
+    if not src.exists():
+        raise ValueError(f'stan file does not exist: {src}')
+
+    compiler_options = CompilerOptions(
+        stanc_options=stanc_options,
+        cpp_options=cpp_options,
+        user_header=user_header,
+    )
+    compiler_options.validate()
+
+    exe_target = src.with_suffix(EXTENSION)
+    if exe_target.exists():
+        exe_time = os.path.getmtime(exe_target)
+        included_files = [src]
+        included_files.extend(
+            src_info(str(src), compiler_options).get('included_files', [])
+        )
+        out_of_date = any(
+            os.path.getmtime(included_file) > exe_time
+            for included_file in included_files
+        )
+        if not out_of_date and not force:
+            get_logger().debug('found newer exe file, not recompiling')
+            return str(exe_target)
+
+    compilation_failed = False
+    # if target path has spaces or special characters, use a copy in a
+    # temporary directory (GNU-Make constraint)
+    with SanitizedOrTmpFilePath(str(src)) as (stan_file, is_copied):
+        exe_file = os.path.splitext(stan_file)[0] + EXTENSION
+
+        hpp_file = os.path.splitext(exe_file)[0] + '.hpp'
+        if os.path.exists(hpp_file):
+            os.remove(hpp_file)
+        if os.path.exists(exe_file):
+            get_logger().debug('Removing %s', exe_file)
+            os.remove(exe_file)
+
+        get_logger().info(
+            'compiling stan file %s to exe file %s',
+            stan_file,
+            exe_target,
+        )
+
+        make = os.getenv(
+            'MAKE',
+            'make' if platform.system() != 'Windows' else 'mingw32-make',
+        )
+        cmd = [make]
+        cmd.extend(compiler_options.compose(filename_in_msg=src.name))
+        cmd.append(Path(exe_file).as_posix())
+
+        sout = io.StringIO()
+        try:
+            do_command(cmd=cmd, cwd=cmdstan_path(), fd_out=sout)
+        except RuntimeError as e:
+            sout.write(f'\n{str(e)}\n')
+            compilation_failed = True
+        finally:
+            console = sout.getvalue()
+
+        get_logger().debug('Console output:\n%s', console)
+        if not compilation_failed:
+            if is_copied:
+                shutil.copy(exe_file, exe_target)
+            get_logger().info('compiled model executable: %s', exe_target)
+        if 'Warning' in console:
+            lines = console.split('\n')
+            warnings = [x for x in lines if x.startswith('Warning')]
+            get_logger().warning(
+                'Stan compiler has produced %d warnings:',
+                len(warnings),
+            )
+            get_logger().warning(console)
+        if compilation_failed:
+            if 'PCH' in console or 'precompiled header' in console:
+                get_logger().warning(
+                    "CmdStan's precompiled header (PCH) files "
+                    "may need to be rebuilt."
+                    "Please run cmdstanpy.rebuild_cmdstan().\n"
+                    "If the issue persists please open a bug report"
+                )
+            raise ValueError(
+                f"Failed to compile Stan model '{src}'. " f"Console:\n{console}"
+            )
+        return str(exe_target)
diff --git a/cmdstanpy/model.py b/cmdstanpy/model.py
index 6bb5ecb..2380b1b 100644
--- a/cmdstanpy/model.py
+++ b/cmdstanpy/model.py
@@ -1,7 +1,5 @@
 """CmdStanModel"""
 
-import io
-import json
 import os
 import platform
 import re
@@ -15,7 +13,6 @@ from concurrent.futures import ThreadPoolExecutor
 from datetime import datetime
 from io import StringIO
 from multiprocessing import cpu_count
-from pathlib import Path
 from typing import (
     Any,
     Callable,
@@ -37,6 +34,7 @@ from cmdstanpy import (
     _CMDSTAN_SAMPLING,
     _CMDSTAN_WARMUP,
     _TMPDIR,
+    compilation,
 )
 from cmdstanpy.cmdstan_args import (
     CmdStanArgs,
@@ -48,7 +46,6 @@ from cmdstanpy.cmdstan_args import (
     SamplerArgs,
     VariationalArgs,
 )
-from cmdstanpy.compiler_opts import CompilerOptions
 from cmdstanpy.stanfit import (
     CmdStanGQ,
     CmdStanLaplace,
@@ -61,7 +58,6 @@ from cmdstanpy.stanfit import (
 )
 from cmdstanpy.utils import (
     EXTENSION,
-    SanitizedOrTmpFilePath,
     cmdstan_path,
     cmdstan_version,
     cmdstan_version_before,
@@ -120,10 +116,12 @@ class CmdStanModel:
         model_name: Optional[str] = None,
         stan_file: OptionalPath = None,
         exe_file: OptionalPath = None,
-        compile: Union[bool, Literal['force']] = True,
+        force_compile: bool = False,
         stanc_options: Optional[Dict[str, Any]] = None,
         cpp_options: Optional[Dict[str, Any]] = None,
         user_header: OptionalPath = None,
+        *,
+        compile: Union[bool, Literal['force'], None] = None,
     ) -> None:
         """
         Initialize object given constructor args.
@@ -140,14 +138,34 @@ class CmdStanModel:
         self._name = ''
         self._stan_file = None
         self._exe_file = None
-        self._compiler_options = CompilerOptions(
+        self._compiler_options = compilation.CompilerOptions(
             stanc_options=stanc_options,
             cpp_options=cpp_options,
             user_header=user_header,
         )
+        self._compiler_options.validate()
+
         self._fixed_param = False
 
+        if compile is None:
+            compile = True
+        else:
+            get_logger().warning(
+                "CmdStanModel(compile=...) is deprecated and will be "
+                "removed in the next major version. The constructor will "
+                "always ensure a model has a compiled executable.\n"
+                "If you wish to force recompilation, use force_compile=True "
+                "instead."
+            )
+
+        if force_compile:
+            compile = 'force'
+
         if model_name is not None:
+            get_logger().warning(
+                "CmdStanModel(model_name=...) is deprecated and will be "
+                "removed in the next major version."
+            )
             if not model_name.strip():
                 raise ValueError(
                     'Invalid value for argument model name, found "{}"'.format(
@@ -156,8 +174,6 @@ class CmdStanModel:
                 )
             self._name = model_name.strip()
 
-        self._compiler_options.validate()
-
         if stan_file is None:
             if exe_file is None:
                 raise ValueError(
@@ -181,12 +197,7 @@ class CmdStanModel:
                 program = fd.read()
             if '#include' in program:
                 path, _ = os.path.split(self._stan_file)
-                if self._compiler_options._stanc_options is None:
-                    self._compiler_options._stanc_options = {
-                        'include-paths': [path]
-                    }
-                else:
-                    self._compiler_options.add_include_path(path)
+                self._compiler_options.add_include_path(path)
 
             # try to detect models w/out parameters, needed for sampler
             if not cmdstan_version_before(
@@ -238,7 +249,7 @@ class CmdStanModel:
                 get_logger().debug("TBB already found in load path")
 
         if compile and self._exe_file is None:
-            self.compile(force=str(compile).lower() == 'force')
+            self.compile(force=str(compile).lower() == 'force', _internal=True)
 
     def __repr__(self) -> str:
         repr = 'CmdStanModel: name={}'.format(self._name)
@@ -299,20 +310,7 @@ class CmdStanModel:
         """
         if self.stan_file is None or cmdstan_version_before(2, 27):
             return {}
-        cmd = (
-            [os.path.join(cmdstan_path(), 'bin', 'stanc' + EXTENSION)]
-            # handle include-paths, allow-undefined etc
-            + self._compiler_options.compose_stanc(None)
-            + ['--info', str(self.stan_file)]
-        )
-        proc = subprocess.run(cmd, capture_output=True, text=True, check=False)
-        if proc.returncode:
-            raise ValueError(
-                f"Failed to get source info for Stan model "
-                f"'{self._stan_file}'. Console:\n{proc.stderr}"
-            )
-        result: Dict[str, Any] = json.loads(proc.stdout)
-        return result
+        return compilation.src_info(str(self.stan_file), self._compiler_options)
 
     def format(
         self,
@@ -443,6 +441,8 @@ class CmdStanModel:
         cpp_options: Optional[Dict[str, Any]] = None,
         user_header: OptionalPath = None,
         override_options: bool = False,
+        *,
+        _internal: bool = False,
     ) -> None:
         """
         Compile the given Stan program file.  Translates the Stan code to
@@ -466,6 +466,13 @@ class CmdStanModel:
         :param override_options: When ``True``, override existing option.
             When ``False``, add/replace existing options.  Default is ``False``.
         """
+        if not _internal:
+            get_logger().warning(
+                "CmdStanModel.compile() is deprecated and will be removed in "
+                "the next major version. To compile a model, use the "
+                "CmdStanModel() constructor or cmdstanpy.compile_stan_file()."
+            )
+
         if not self._stan_file:
             raise RuntimeError('Please specify source file')
 
@@ -475,7 +482,7 @@ class CmdStanModel:
             or cpp_options is not None
             or user_header is not None
         ):
-            compiler_options = CompilerOptions(
+            compiler_options = compilation.CompilerOptions(
                 stanc_options=stanc_options,
                 cpp_options=cpp_options,
                 user_header=user_header,
@@ -490,86 +497,14 @@ class CmdStanModel:
                     self._compiler_options = compiler_options
                 else:
                     self._compiler_options.add(compiler_options)
-        exe_target = os.path.splitext(self._stan_file)[0] + EXTENSION
-        if os.path.exists(exe_target):
-            exe_time = os.path.getmtime(exe_target)
-            included_files = [self._stan_file]
-            included_files.extend(self.src_info().get('included_files', []))
-            out_of_date = any(
-                os.path.getmtime(included_file) > exe_time
-                for included_file in included_files
-            )
-            if not out_of_date and not force:
-                get_logger().debug('found newer exe file, not recompiling')
-                if self._exe_file is None:  # called from constructor
-                    self._exe_file = exe_target
-                return
-
-        compilation_failed = False
-        # if target path has spaces or special characters, use a copy in a
-        # temporary directory (GNU-Make constraint)
-        with SanitizedOrTmpFilePath(self._stan_file) as (stan_file, is_copied):
-            exe_file = os.path.splitext(stan_file)[0] + EXTENSION
-
-            hpp_file = os.path.splitext(exe_file)[0] + '.hpp'
-            if os.path.exists(hpp_file):
-                os.remove(hpp_file)
-            if os.path.exists(exe_file):
-                get_logger().debug('Removing %s', exe_file)
-                os.remove(exe_file)
 
-            get_logger().info(
-                'compiling stan file %s to exe file %s',
-                self._stan_file,
-                exe_target,
-            )
-
-            make = os.getenv(
-                'MAKE',
-                'make' if platform.system() != 'Windows' else 'mingw32-make',
-            )
-            cmd = [make]
-            if self._compiler_options is not None:
-                cmd.extend(self._compiler_options.compose(self._stan_file))
-            cmd.append(Path(exe_file).as_posix())
-
-            sout = io.StringIO()
-            try:
-                do_command(cmd=cmd, cwd=cmdstan_path(), fd_out=sout)
-            except RuntimeError as e:
-                sout.write(f'\n{str(e)}\n')
-                compilation_failed = True
-            finally:
-                console = sout.getvalue()
-
-            get_logger().debug('Console output:\n%s', console)
-            if not compilation_failed:
-                if is_copied:
-                    shutil.copy(exe_file, exe_target)
-                self._exe_file = exe_target
-                get_logger().info(
-                    'compiled model executable: %s', self._exe_file
-                )
-            if 'Warning' in console:
-                lines = console.split('\n')
-                warnings = [x for x in lines if x.startswith('Warning')]
-                get_logger().warning(
-                    'Stan compiler has produced %d warnings:',
-                    len(warnings),
-                )
-                get_logger().warning(console)
-            if compilation_failed:
-                if 'PCH' in console or 'precompiled header' in console:
-                    get_logger().warning(
-                        "CmdStan's precompiled header (PCH) files "
-                        "may need to be rebuilt."
-                        "Please run cmdstanpy.rebuild_cmdstan().\n"
-                        "If the issue persists please open a bug report"
-                    )
-                raise ValueError(
-                    f"Failed to compile Stan model '{self._stan_file}'. "
-                    f"Console:\n{console}"
-                )
+        self._exe_file = compilation.compile_stan_file(
+            str(self.stan_file),
+            force=force,
+            stanc_options=self._compiler_options.stanc_options,
+            cpp_options=self._compiler_options.cpp_options,
+            user_header=self._compiler_options.user_header,
+        )
 
     def optimize(
         self,


Test patch:
diff --git a/test/test_compiler_opts.py b/test/test_compiler_opts.py
index a9cd3e5..02a57a9 100644
--- a/test/test_compiler_opts.py
+++ b/test/test_compiler_opts.py
@@ -6,7 +6,7 @@ from test import check_present
 
 import pytest
 
-from cmdstanpy.compiler_opts import CompilerOptions
+from cmdstanpy.compilation import CompilerOptions
 
 HERE = os.path.dirname(os.path.abspath(__file__))
 DATAFILES_PATH = os.path.join(HERE, 'data')
diff --git a/test/test_model.py b/test/test_model.py
index 6486bbc..55c2bfe 100644
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -281,7 +281,7 @@ def test_compile_with_includes(
     # Compile after modifying included file, ensuring cache is not used.
     def _patched_getmtime(filename: str) -> float:
         includes = ['divide_real_by_two.stan', 'add_one_function.stan']
-        if any(filename.endswith(include) for include in includes):
+        if any(str(filename).endswith(include) for include in includes):
             return float('inf')
         return getmtime(filename)