macOS force arch

[maedoc]

Submission Checklist

• Run unit tests
• Declare copyright holder and open-source license: see below

Summary

This is a WIP to force the architecture for invoking the Make toolchain on macOS, to avoid annoying errors about incompatible PCH files, when running (a) make from arm64 on a x86_64 compiled cmdstan and (b) vice versa.

This is WIP w/o tests yet to ask for initial feedback: (a) is this useful/desirable and (b) does it want full tests (i.e. building two arch copies) or can exist on a best effort basis?

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): myself

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)

[WardBrian]

I think a best-effort basis is what we've done for some similar hackery on Windows. It's almost impossible to test this in CI.

Is there no way to do this other than trying to run g++ on something (also, shouldn't this be clang, or better, os.environ.get('CXX', 'clang')?

[maedoc]

I thought pch files were Clang and gch files were GCC, and I found a gch file, but yes I can test. I couldn’t file a cli tool to dump the target of a pch file, so I resorted to invoking it directly. I’ll look at checking bin/stansummary or similar since it’s also arch dependent (while stanc is x86 in any case I think).

I’ll get it looking a bit nicer and tested locally at least and then undraft the PR.

[bob-carpenter]

Hi, @maedoc. I just wanted to say thanks again, given that we started from your cmdstan wrapper.

Of course, we'll be eager to merge anything that reduces the number of PCH errors we get.

[maedoc]

I've simplified the arch test by just checking what file bin/stansummary says (either .Mach-O 64-bit executable x86_64 or Mach-O 64-bit executable arm64), and here's it working in my case, calling from an arm64 kernel to a x86_64 compiled CmdStan,

[WardBrian]

This LGTM. I don't think we have any solid way of testing this (see: open request for M1 runners actions/runner-images#2187), but if you're able to confirm it works locally I think that this is the right move.

Do you happen to know if arch also will effect which C++ compilers get used, etc? E.g., if I have my CXX variable set to an x86 compiler running under rosetta, what will this change do?

[maedoc]

Good point 👍 If the arch of the argument to arch doesn’t match the requested arch then it fails. I can add a check for the compiler arch.

[WardBrian]

Yeah, unsure how to handle this. I think a simple check that says if the arch of the compiler doesn't match the arch of stansummary, fail with a nice error message? This should only trigger if CXX is set, I think

[maedoc]

Actually, if make is the only command being invoked directly, then setting CXX to the wrong arch won't be a problem: make itself is a so-called universal binary with both architectures (as is the platform c++), and then subprocess calls from make handle the invocation across architectures w/o problem. To wit, a test makefile like this

all:
	file $(shell which $(CXX))
	$(CXX) -v

with invocations like

• make: defaults to arm64
• arch -x86_64 make: make runs as x86_64, c++ is invoked as x86_64
• arch -arm64 make: make runs as arm64, c++ is invoked as arm64
• arch -x86_64 make CXX=/opt/homebrew/bin/g++-11: make runs as x86_64, invokes an arm64 version of g++
• arch -arm64 make CXX=/usr/local/bin/g++-11: make runs as arm64, invokes an x86_64 version of g++

all produce the right output.

In summary, I think the implemented approach is sound and macOS' behavior is kind-of sane, and so we avoid just the issue of a mismatch between the compiled CmdStan arch and the implicit arch when invoking make from Python.

edit for formatting

[WardBrian]

I guess the concern is if they installed cmdstan with something set for CXX that is arm64-based, and then later are trying to use a CXX which is x86, won't that lead to the PCH error this PR is trying to avoid?

[maedoc]

Ah I see what you mean. Like building CmdStan with CXX=/opt/homebrew/bin/g++-11 and compiling a model with CXX=/usr/local/bin/g++-11? You're right that the current PR assumes the platform compiler and wouldn't behave correctly when mixing CXX archs.

[WardBrian]

It seems like an easy thing to detect and (at least) issue a warning about, though. Would you mind adding that?

[maedoc]

Sure, can you link to an example? I couldn't find anything in the codebase which shows how an CXX is specified. I thought it'd be cpp_options but it's typed as Dict[str, Union[bool, int]].

[WardBrian]

I'm seeing the definition of cpp_options as Dict[str,Any] - where are you seeing that other type? It is wrong if it is in the code somewhere.

I think we need to check both cpp_options and os.environ('CXX'), in cases where the user did something like
CXX=something python or is using a conda environment which can set CXX

[maedoc]

where are you seeing that other type?

Here for instance. Is that a typo or it's something else? I can add the check for env var as well yes.

[WardBrian]

Yeah that is incorrect. I think the semantics of mypy’s typing mean that it doesn’t notice that because elsewhere it is declared to be Any

[maedoc]

I don't currently have time to finish this (before September at earliest), in case someone else wants to pick it up.

[WardBrian]

I’m not sure if we have a cmdstanpy dev with an M1/2 machine, so happy to wait. We can push this out of the 1.0.2 milestone