From 16fccb6a1ea2c541199e1d12251cedab0839eb5a Mon Sep 17 00:00:00 2001 From: Mitzi Morris Date: Tue, 4 Aug 2020 14:36:09 -0400 Subject: [PATCH 1/6] tightened up code and comments, added tests --- cmdstanpy/model.py | 16 ++++++++++++++-- test/test_model.py | 15 +++++++++++---- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/cmdstanpy/model.py b/cmdstanpy/model.py index 1ee30b69..f0563b09 100644 --- a/cmdstanpy/model.py +++ b/cmdstanpy/model.py @@ -50,6 +50,8 @@ class CmdStanModel: model given data. - By default, compiles model on instantiation - override with argument ``compile=False`` + - By default, property ``name`` corresponds to basename of the Stan program + or exe file - override with argument ``model_name=``. """ def __init__( @@ -72,6 +74,12 @@ def __init__( self._logger = logger or get_logger() if model_name is not None: + if len(model_name.strip()) == 0: + raise ValueError( + 'Bad value for argument model name, found "{}"'.format( + model_name + ) + ) self._name = model_name.strip() if stan_file is None: @@ -158,9 +166,13 @@ def __repr__(self) -> str: @property def name(self) -> str: - """Stan program name; corresponds to bare filename, no extension.""" + """ + Stan program name. + Set by the constructor to value of argument `model_name`; if not specified + will be basename of Stan program file, if specified, else exe file. + """ return self._name - + @property def stan_file(self) -> str: """Full path to Stan program file.""" diff --git a/test/test_model.py b/test/test_model.py index fc563fbb..91f12a59 100644 --- a/test/test_model.py +++ b/test/test_model.py @@ -31,7 +31,7 @@ BERN_STAN = os.path.join(DATAFILES_PATH, 'bernoulli.stan') BERN_EXE = os.path.join(DATAFILES_PATH, 'bernoulli' + EXTENSION) - +BERN_BASENAME = 'bernoulli' class CmdStanModelTest(unittest.TestCase): @@ -50,22 +50,24 @@ def show_cmdstan_version(self): self.assertTrue(True) def test_model_good(self): - # compile on instantiation + # compile on instantiation, override model name model = CmdStanModel(model_name='bern', stan_file=BERN_STAN) self.assertEqual(BERN_STAN, model.stan_file) self.assertTrue(model.exe_file.endswith(BERN_EXE.replace('\\', '/'))) self.assertEqual('bern', model.name) + # default model name + model = CmdStanModel(stan_file=BERN_STAN) + self.assertEqual(BERN_BASENAME, model.name) + # instantiate with existing exe model = CmdStanModel(stan_file=BERN_STAN, exe_file=BERN_EXE) self.assertEqual(BERN_STAN, model.stan_file) self.assertTrue(model.exe_file.endswith(BERN_EXE)) - self.assertEqual('bernoulli', model.name) # instantiate with existing exe only - no model model2 = CmdStanModel(exe_file=BERN_EXE) self.assertEqual(BERN_EXE, model2.exe_file) - self.assertEqual('bernoulli', model2.name) with self.assertRaises(RuntimeError): model2.code() with self.assertRaises(RuntimeError): @@ -77,11 +79,16 @@ def test_model_good(self): self.assertEqual(BERN_STAN, model.stan_file) self.assertEqual(None, model.exe_file) + def test_model_bad(self): with self.assertRaises(ValueError): CmdStanModel(stan_file=None, exe_file=None) with self.assertRaises(ValueError): CmdStanModel(model_name='bad') + with self.assertRaises(ValueError): + CmdStanModel(model_name='', stan_file=BERN_STAN) + with self.assertRaises(ValueError): + CmdStanModel(model_name=' ', stan_file=BERN_STAN) def test_stanc_options(self): opts = { From 4a5a3eff1b519c6456d6b8f4ccb4373f9cf47202 Mon Sep 17 00:00:00 2001 From: Mitzi Morris Date: Tue, 4 Aug 2020 14:36:36 -0400 Subject: [PATCH 2/6] tightened up code and comments, added tests --- cmdstanpy/model.py | 2 +- test/test_model.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmdstanpy/model.py b/cmdstanpy/model.py index f0563b09..2970123e 100644 --- a/cmdstanpy/model.py +++ b/cmdstanpy/model.py @@ -172,7 +172,7 @@ def name(self) -> str: will be basename of Stan program file, if specified, else exe file. """ return self._name - + @property def stan_file(self) -> str: """Full path to Stan program file.""" diff --git a/test/test_model.py b/test/test_model.py index 91f12a59..914aaba1 100644 --- a/test/test_model.py +++ b/test/test_model.py @@ -33,6 +33,7 @@ BERN_EXE = os.path.join(DATAFILES_PATH, 'bernoulli' + EXTENSION) BERN_BASENAME = 'bernoulli' + class CmdStanModelTest(unittest.TestCase): # pylint: disable=no-self-use @@ -79,7 +80,6 @@ def test_model_good(self): self.assertEqual(BERN_STAN, model.stan_file) self.assertEqual(None, model.exe_file) - def test_model_bad(self): with self.assertRaises(ValueError): CmdStanModel(stan_file=None, exe_file=None) From c4b2ba7c5db873ce2fd7a46a16b2234a31c3ba68 Mon Sep 17 00:00:00 2001 From: Mitzi Morris Date: Tue, 4 Aug 2020 14:44:32 -0400 Subject: [PATCH 3/6] tightened up code and comments, added tests --- cmdstanpy/model.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmdstanpy/model.py b/cmdstanpy/model.py index 2970123e..b883a327 100644 --- a/cmdstanpy/model.py +++ b/cmdstanpy/model.py @@ -167,9 +167,9 @@ def __repr__(self) -> str: @property def name(self) -> str: """ - Stan program name. - Set by the constructor to value of argument `model_name`; if not specified - will be basename of Stan program file, if specified, else exe file. + Model name used in output filename templates. Default is basename + of Stan program or exe file, unless specified in call to constructor + via argument `model_name`. """ return self._name From 62a6f8c1e06825dae41d07f61465fcf4f2e888b8 Mon Sep 17 00:00:00 2001 From: Ari Hartikainen Date: Tue, 4 Aug 2020 22:21:30 +0300 Subject: [PATCH 4/6] Update and refine windows tools (#257) * update windows compiler handling * change compiler order Co-authored-by: ahartikainen --- cmdstanpy/install_cmdstan.py | 32 ++++++++++++++ cmdstanpy/install_cxx_toolchain.py | 10 +++-- cmdstanpy/utils.py | 69 ++++++++++++++++++------------ 3 files changed, 80 insertions(+), 31 deletions(-) diff --git a/cmdstanpy/install_cmdstan.py b/cmdstanpy/install_cmdstan.py index 82a42922..4324b1bb 100644 --- a/cmdstanpy/install_cmdstan.py +++ b/cmdstanpy/install_cmdstan.py @@ -4,6 +4,7 @@ Optional command line arguments: -v, --version : version, defaults to latest -d, --dir : install directory, defaults to '~/.cmdstanpy + -c --compiler : add C++ compiler to path (Windows only) """ import argparse import contextlib @@ -186,6 +187,13 @@ def main(): parser = argparse.ArgumentParser() parser.add_argument('--version', '-v') parser.add_argument('--dir', '-d') + if platform.system() == 'Windows': + # use compiler installed with install_cxx_toolchain + # Install a new compiler if compiler not found + # Search order is RTools40, RTools35 + parser.add_argument( + '--compiler', '-c', dest='compiler', action='store_true' + ) args = parser.parse_args(sys.argv[1:]) version = vars(args)['version'] @@ -199,6 +207,30 @@ def main(): validate_dir(install_dir) print('Install directory: {}'.format(install_dir)) + if platform.system() == 'Windows' and vars(args)['compiler']: + from .install_cxx_toolchain import ( + main as _main_cxx, + is_installed as _is_installed_cxx, + ) + from .utils import cxx_toolchain_path + + cxx_loc = os.path.expanduser(os.path.join('~', '.cmdstanpy')) + compiler_found = False + for cxx_version in ['40', '35']: + if _is_installed_cxx(cxx_loc, cxx_version): + compiler_found = True + break + if not compiler_found: + print('Installing RTools40') + # copy argv and clear sys.argv + original_argv = sys.argv[:] + sys.argv = sys.argv[:1] + _main_cxx() + sys.argv = original_argv + cxx_version = '40' + # Add toolchain to $PATH + cxx_toolchain_path(cxx_version) + cmdstan_version = 'cmdstan-{}'.format(version) with pushd(install_dir): if not os.path.exists(cmdstan_version): diff --git a/cmdstanpy/install_cxx_toolchain.py b/cmdstanpy/install_cxx_toolchain.py index 1500ac8a..3fe3ba63 100644 --- a/cmdstanpy/install_cxx_toolchain.py +++ b/cmdstanpy/install_cxx_toolchain.py @@ -110,8 +110,12 @@ def install_mingw32_make(toolchain_loc): list( OrderedDict.fromkeys( [ + os.path.join( + toolchain_loc, + 'mingw_64' if IS_64BITS else 'mingw_32', + 'bin', + ), os.path.join(toolchain_loc, 'usr', 'bin'), - os.path.join(toolchain_loc, 'mingw64', 'bin'), ] + os.environ.get('PATH', '').split(';') ) @@ -152,7 +156,7 @@ def install_mingw32_make(toolchain_loc): def is_installed(toolchain_loc, version): """Returns True is toolchain is installed.""" if platform.system() == 'Windows': - if version == '3.5': + if version in ['35', '3.5']: if not os.path.exists(os.path.join(toolchain_loc, 'bin')): return False return os.path.exists( @@ -163,7 +167,7 @@ def is_installed(toolchain_loc, version): 'g++' + EXTENSION, ) ) - elif version == '4.0': + elif version in ['40', '4.0', '4']: return os.path.exists( os.path.join( toolchain_loc, diff --git a/cmdstanpy/utils.py b/cmdstanpy/utils.py index d4a704a4..6e60e6a9 100644 --- a/cmdstanpy/utils.py +++ b/cmdstanpy/utils.py @@ -214,112 +214,125 @@ def cxx_toolchain_path(version: str = None) -> Tuple[str]: toolchain_root = '' if 'CMDSTAN_TOOLCHAIN' in os.environ: toolchain_root = os.environ['CMDSTAN_TOOLCHAIN'] - if os.path.exists(os.path.join(toolchain_root, 'mingw_64')): + if os.path.exists(os.path.join(toolchain_root, 'mingw64')): compiler_path = os.path.join( toolchain_root, - 'mingw_64' if (sys.maxsize > 2 ** 32) else 'mingw_32', + 'mingw64' if (sys.maxsize > 2 ** 32) else 'mingw32', 'bin', ) if os.path.exists(compiler_path): - tool_path = os.path.join(toolchain_root, 'bin') + tool_path = os.path.join(toolchain_root, 'usr', 'bin') if not os.path.exists(tool_path): tool_path = '' compiler_path = '' logger.warning( - 'Found invalid installion for RTools35 on %s', + 'Found invalid installion for RTools40 on %s', toolchain_root, ) toolchain_root = '' else: compiler_path = '' logger.warning( - 'Found invalid installion for RTools35 on %s', + 'Found invalid installion for RTools40 on %s', toolchain_root, ) toolchain_root = '' - elif os.path.exists(os.path.join(toolchain_root, 'mingw64')): + + elif os.path.exists(os.path.join(toolchain_root, 'mingw_64')): compiler_path = os.path.join( toolchain_root, - 'mingw64' if (sys.maxsize > 2 ** 32) else 'mingw32', + 'mingw_64' if (sys.maxsize > 2 ** 32) else 'mingw_32', 'bin', ) if os.path.exists(compiler_path): - tool_path = os.path.join(toolchain_root, 'usr', 'bin') + tool_path = os.path.join(toolchain_root, 'bin') if not os.path.exists(tool_path): tool_path = '' compiler_path = '' logger.warning( - 'Found invalid installion for RTools40 on %s', + 'Found invalid installion for RTools35 on %s', toolchain_root, ) toolchain_root = '' else: compiler_path = '' logger.warning( - 'Found invalid installion for RTools40 on %s', + 'Found invalid installion for RTools35 on %s', toolchain_root, ) toolchain_root = '' else: rtools_dir = os.path.expanduser( - os.path.join('~', '.cmdstanpy', 'RTools') + os.path.join('~', '.cmdstanpy', 'RTools40') ) if not os.path.exists(rtools_dir): - raise ValueError( - 'no RTools installation found, ' - 'run command line script "install_cxx_toolchain"' + rtools_dir = os.path.expanduser( + os.path.join('~', '.cmdstanpy', 'RTools35') ) + if not os.path.exists(rtools_dir): + rtools_dir = os.path.expanduser( + os.path.join('~', '.cmdstanpy', 'RTools') + ) + if not os.path.exists(rtools_dir): + raise ValueError( + 'no RTools installation found, ' + 'run command line script "install_cxx_toolchain"' + ) + else: + rtools_dir = os.path.expanduser(os.path.join('~', '.cmdstanpy')) + else: + rtools_dir = os.path.expanduser(os.path.join('~', '.cmdstanpy')) compiler_path = '' tool_path = '' - if version not in ('4', '40', '4.0') and os.path.exists( - os.path.join(rtools_dir, 'RTools35') + if version not in ('35', '3.5', '3') and os.path.exists( + os.path.join(rtools_dir, 'RTools40') ): - toolchain_root = os.path.join(rtools_dir, 'RTools35') + toolchain_root = os.path.join(rtools_dir, 'RTools40') compiler_path = os.path.join( toolchain_root, - 'mingw_64' if (sys.maxsize > 2 ** 32) else 'mingw_32', + 'mingw64' if (sys.maxsize > 2 ** 32) else 'mingw32', 'bin', ) if os.path.exists(compiler_path): - tool_path = os.path.join(toolchain_root, 'bin') + tool_path = os.path.join(toolchain_root, 'usr', 'bin') if not os.path.exists(tool_path): tool_path = '' compiler_path = '' logger.warning( - 'Found invalid installion for RTools35 on %s', + 'Found invalid installation for RTools40 on %s', toolchain_root, ) toolchain_root = '' else: compiler_path = '' logger.warning( - 'Found invalid installion for RTools35 on %s', + 'Found invalid installation for RTools40 on %s', toolchain_root, ) toolchain_root = '' if ( not toolchain_root or version in ('4', '40', '4.0') - ) and os.path.exists(os.path.join(rtools_dir, 'RTools40')): - toolchain_root = os.path.join(rtools_dir, 'RTools40') + ) and os.path.exists(os.path.join(rtools_dir, 'RTools35')): + toolchain_root = os.path.join(rtools_dir, 'RTools35') compiler_path = os.path.join( toolchain_root, - 'mingw64' if (sys.maxsize > 2 ** 32) else 'mingw32', + 'mingw_64' if (sys.maxsize > 2 ** 32) else 'mingw_32', 'bin', ) if os.path.exists(compiler_path): - tool_path = os.path.join(toolchain_root, 'usr', 'bin') + tool_path = os.path.join(toolchain_root, 'bin') if not os.path.exists(tool_path): tool_path = '' compiler_path = '' logger.warning( - 'Found invalid installion for RTools40 on %s', + 'Found invalid installation for RTools35 on %s', toolchain_root, ) toolchain_root = '' else: compiler_path = '' logger.warning( - 'Found invalid installion for RTools40 on %s', + 'Found invalid installation for RTools35 on %s', toolchain_root, ) toolchain_root = '' @@ -328,7 +341,7 @@ def cxx_toolchain_path(version: str = None) -> Tuple[str]: 'no C++ toolchain installation found, ' 'run command line script "install_cxx_toolchain"' ) - logger.info('Adds C++ toolchain to $PATH: %s', toolchain_root) + logger.info('Add C++ toolchain to $PATH: %s', toolchain_root) os.environ['PATH'] = ';'.join( list( OrderedDict.fromkeys( From c875cc8a091648c49a4f894a00514d14e2032f35 Mon Sep 17 00:00:00 2001 From: Mitzi Morris Date: Tue, 4 Aug 2020 17:09:04 -0400 Subject: [PATCH 5/6] Update cmdstanpy/model.py Co-authored-by: Ari Hartikainen --- cmdstanpy/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmdstanpy/model.py b/cmdstanpy/model.py index b883a327..39f43aef 100644 --- a/cmdstanpy/model.py +++ b/cmdstanpy/model.py @@ -74,7 +74,7 @@ def __init__( self._logger = logger or get_logger() if model_name is not None: - if len(model_name.strip()) == 0: + if not model_name.strip(): raise ValueError( 'Bad value for argument model name, found "{}"'.format( model_name From f3e499478739071dec2bdf79ff3a1df3cfa76e35 Mon Sep 17 00:00:00 2001 From: Mitzi Morris Date: Tue, 4 Aug 2020 17:09:19 -0400 Subject: [PATCH 6/6] Update cmdstanpy/model.py Co-authored-by: Ari Hartikainen --- cmdstanpy/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmdstanpy/model.py b/cmdstanpy/model.py index 39f43aef..39c8b2ed 100644 --- a/cmdstanpy/model.py +++ b/cmdstanpy/model.py @@ -76,7 +76,7 @@ def __init__( if model_name is not None: if not model_name.strip(): raise ValueError( - 'Bad value for argument model name, found "{}"'.format( + 'Invalid value for argument model name, found "{}"'.format( model_name ) )