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

Missing support for Apple Silicon architecture on M1/M2 Mac #80

Open
ehsanhaghighat opened this issue Jun 3, 2022 · 14 comments
Open

Missing support for Apple Silicon architecture on M1/M2 Mac #80

ehsanhaghighat opened this issue Jun 3, 2022 · 14 comments
Labels
feature Proposal for a new feature.

Comments

@ehsanhaghighat
Copy link

Thanks for the tool - I found that the import doesn't handle COMSOL6/macarm64.

@john-hen
Copy link
Collaborator

john-hen commented Jun 6, 2022

Hi, thanks for bringing this up. I don't have access to an M1 Mac, so can't test any of this. The fix you're suggesting in the title, do you mean to replace any occurrence of "maci64" with "macarm64" in discovery.py?

Also, I suppose you used the "universal2" Python installers for macOS, so this is running natively on Apple Silicon, not through Rosetta. And when you installed Comsol 6, could you choose between the two variants? In other words, is there a chance Comsol could either run natively or through the emulation layer on the M1 Mac? Probably shouldn't matter for the discovery mechanism, just trying to get a feel if there's a way we might break backward compatibility.

@doub1emint
Copy link

I have the same problem, and I replace any occurrence of "maci64" with "macarm64" in discovery.py. it worked!
thanks for effective suggestions.

@max3-2
Copy link
Contributor

max3-2 commented Jun 22, 2022

In other words, is there a chance Comsol could either run natively or through the emulation layer on the M1 Mac? Probably shouldn't matter for the discovery mechanism, just trying to get a feel if there's a way we might break backward compatibility.

COMSOL states on their site for Apple Sillicon that is is a good practice to

Generally, install both the native (M1) and Intel versions of COMSOL and use the version that suits you best.

So I assume that you can install both in parallel and the discovery will show both, as soon as the second search would be implemented. How (if) is a priorization implemented if not on Version number? So what would be the default on macs with both options installed?

I sadly do not have unconditional access to COMSOL anymore so I can just assume some things. Still waiting on a project where I can justify using it

@john-hen
Copy link
Collaborator

Ah, okay. That's a good hint, was not aware of that. And yes, that is a complication because it's not clear which one to choose. Would make sense to select depending on which Python is running, but I don't know exactly (yet) how to do that. I'm starting a new job soon. Won't be working with Comsol, but will have a Mac M1 as my developer machine, and perhaps (not guaranteed) access to a Comsol campus license. So will be looking into it then, though in due time obviously. Also, if both can be installed and is even recommended, it should actually work, I would assume, if that recommendation is followed. Though obviously, it would be preferable to use the Apple Silicon variant.

@john-hen
Copy link
Collaborator

john-hen commented Aug 1, 2022

So, turns out, my "new Mac" isn't all that new, it's still Intel-based. I can test on that, but not on Apple Silicon. But I do have a campus license, including the latest Comsol version. If anyone wants to submit a PR that handles both system architectures on a Mac, I could help out with testing, with that limitation in mind. Otherwise it's unlikely I'll fix this issue myself anytime soon.

@john-hen john-hen added the bug Something isn't working. label Aug 12, 2022
@silveira-lucas
Copy link

I have the same problem, and I replace any occurrence of "maci64" with "macarm64" in discovery.py. it worked! thanks for effective suggestions.

Hi doub1emint,
I tried your suggestion without success. I changed every instance of the word "maci64" for "macarm64", even in the comments. I also searched for it in the other package files. However, strange enough, I still the error message which mentioning "maci64". I appreciate any help, please pleaaaase :)

@doub1emint
Copy link

Hi, silveira,
I'm sorry to hear that and I have no idea about your problem.

@john-hen john-hen added the needs info More information is needed. label Aug 27, 2022
@john-hen john-hen changed the title Import fails on M1 mac --> fix by changing maci64 to macarm64 Import fails on M1 Mac Aug 28, 2022
@john-hen john-hen removed the bug Something isn't working. label Aug 28, 2022
@john-hen john-hen changed the title Import fails on M1 Mac Import fails on Apple Silicon (M1/M2) Mac Apr 21, 2023
john-hen added a commit that referenced this issue May 2, 2023
@john-hen john-hen changed the title Import fails on Apple Silicon (M1/M2) Mac Missing support for Apple Silicon architecture on M1/M2 Mac May 2, 2023
@pnorgaard
Copy link

I'm running on an Apple M1 Pro, OS 13.4.1, and found that doub1emint's suggestion worked for me. Looks like "maci64" doesn't need to be replaced though, just add "macarm64" to the architectures list on line 42 of discovery.py. Will write a pull request.

@john-hen
Copy link
Collaborator

john-hen commented Jul 7, 2023

Hi Peter (@pnorgaard ), please note that this is such an easy code change because I made it so. The problem here is not discovering the installation. It's testing, documenting, and thinking this through.

This will be a case where we have two different architectures on the same platform. (Kind of like when there were 32-bit and 64-bit versions of Comsol on Windows/Linux. Though Comsol dropped support for 32-bit before MPh was even created.) As I understand it (and Comsol's documentation on this is minimal), people could have both, the Intel-version and the ARM-based version of Comsol, installed at the same time. There is the Rosetta emulation layer that makes this possible. They could also be running Python through Rosetta, I believe. Anyway, lots of things can go wrong there, and I'm in no position to support users on ARM-based Macs at all.

@pnorgaard
Copy link

Understood - yes, I figured the iteration design was intention for such situations, and it did indeed make this quite easy. I was pleasantly surprised that the library behaved quite well with only modification to discovery.py.

If you have recommendation for what kinds of testing provides good coverage, then I'm glad to test on an ARM-based Mac with both the ARM and Intel + Rosetta emulation versions. I might skip on the added fun of python via Rosetta emulation, which feels pretty edge case, at least to me.

I'll revert the pull request, since this chat should at least give users a sharp edged work around.

@john-hen
Copy link
Collaborator

john-hen commented Jul 7, 2023

Sounds great! 😄

I have some suggestions on how to move forward, but would get back to you with more details some other time. And on that note: Take your time whenever you're busy. We all are.

For the record, I haven't seen a pull request. So I don't think you've created one. (Though I did see you created a fork, with a new branch, and the commit you added there.) Rest assured, I'll merge it eventually once we've figured this out. The more contributors, the merrier.

Most importantly, I need "eyes" on the ARM-based Macs. One thing, for example, that the Comsol documentation does not answer: When you run the installer on your machine, does it give you the option to install: the ARM-based Comsol version, or the Intel-based Comsol version, or both? For Comsol 6.1, that is. Any idea what this was like for Comsol 6.0? They may have changed the installer's default behavior, because ARM was not officially supported until 6.1.

I agree that running Python via Rosetta is an afterthought. We should assume people run Python natively - using the "universal installer", if that's the correct term. But we may have to make that explicit in the "Installation" chapter of the documentation for macOS users.

You should run the test suite on your machine. I "hope" that the various ReadMe files, such as the ones in the tests and tools folder, are helpful enough. If not, I'll explain, and eventually amend them.

@fengxiaot
Copy link

fengxiaot commented Jul 25, 2023

I'm running the mph package on Apple M2. pnorgaard's suggestion also worked for me. My COMSOL version is 6.1. Just add "macarm64" to the architectures list. No need to replace "maci64".

@pnorgaard
Copy link

pnorgaard commented Jul 28, 2023 via email

@john-hen
Copy link
Collaborator

@pnorgaard
Okay, so that means, if I understand that correctly, that you can have both Comsol versions (the ARM-based one and the Intel one) installed in parallel on an ARM-based Mac.

The primary concern here is backward compatibility. So if a new version of MPh is released with support for ARM, it shouldn't break existing installations using the Intel-based Comsol, on either Intel-based or ARM-based devices. And it's only the ARM-based Macs that are a concern. (Intel Macs have been tested.)

Plus, with Comsol 6.0 (but not 6.1), the Intel version was actually recommended, as the ARM version still lacked features, I believe. But we can leave it to the user to make that decision and install the one they want. That is, they'd have to uninstall the Intel-Comsol if they want to use the ARM-based one. (Or rename the folder or something, to hide it from the discovery mechanism.) The documentation then must make it clear that the Intel variant (for a given Comsol version) will be selected (if properly installed), and that the ARM variant will be silently ignored.

In the code, it means we search [maci64, macarm64], in that order, as you suggest. We will assume (as we do now) that Comsol is installed in its default location. Complications that may arise from putting it elsewhere, but possibly having comsol on the search path, are left for the user to deal with.

The documentation should also point out that the support for ARM is "experimental". That's based on the warnings that you report. I have no idea what they mean and where they are issued. Not by MPh, for sure. But this does not affect backward compatibility, so it's fine.

You should still run the entire test suite, not just mph.start(). So that we have a clear picture of what's going on, based on maximum code coverage. Ideally, for Comsol 6.0 as well. And for both Comsol architectures, of course. The test suite should pass without any warnings (I hope) with the Intel-based Comsol. And we want to see if more problems pop up with the ARM variant(s). Hopefully not. 🤞

TermeHansen pushed a commit to resolventdk/MPh that referenced this issue Aug 3, 2023
@john-hen john-hen added feature Proposal for a new feature. and removed needs info More information is needed. labels Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposal for a new feature.
Projects
None yet
Development

No branches or pull requests

7 participants