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

[DOC][BUG] Add warning regarding issues with macOS ARM #4010

Merged
merged 10 commits into from
Dec 31, 2022
Merged

[DOC][BUG] Add warning regarding issues with macOS ARM #4010

merged 10 commits into from
Dec 31, 2022

Conversation

dainelli98
Copy link
Contributor

Reference Issues/PRs

Fixes #3994.

What does this implement/fix? Explain your changes.

This PR adds a warning for macOS users using ARM macs of the issues that they may encounter when installing all_extras, related to some dependencies not compatible with that kind of setup.

Does your contribution introduce a new dependency? If yes, which one?

This contribution does not add any new dependency.

What should a reviewer concentrate their feedback on?

Check if the warning is enough, or it would be appropriate to add a link to a separate new article in the documentation explaining in more detail the issue, listing the dependencies that cause the error, in case the user wants to install all the other dependencies in all_extras that do not generate any errors.

PR checklist

For all contributions
  • I've added myself to the list of contributors.
  • The PR title starts with either [ENH], [MNT], [DOC], or [BUG] indicating whether the PR topic is related to enhancement, maintenance, documentation, or bug.

Add missing dot at the end of warning regarding pre-existing conda environment with the same name as the one suggested in installation instructions.
Add myself, @dainelli98, to list of contributors in .all_contributorsrc.
Add warning in installation documentation explaining that Some of the dependencies included in ``all_extras`` do not work on mac ARM-based processors, such as M1, M1Pro, M1Max or M1Ultra.
@dainelli98 dainelli98 marked this pull request as ready for review December 28, 2022 17:44
fkiraly
fkiraly previously approved these changes Dec 28, 2022
Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, great!

If you have a workaround that can be added, that would also be nice, but it´s not blocking.

Add workaround in troubleshooting section explaining which libraries do not work on macOS ARM devices and which ones should be avoided or replaced to keep the maximum amount of dependencies from ``all_extras``.
…ion extensions.

Add ``sphinx.ext.autosectionlabel`` to ``conf.py`` extensions to enable the possibility to add
links to sections in documentation.
fkiraly
fkiraly previously approved these changes Dec 31, 2022
Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thanks!
I don't think the troubleshooting paragraph is too long, it is clear and helpful.

No blocking comments from my side.

I left some comments on how you could improve formulations - in tech writing and espceially tech instructions, it's worth being very clear and precise (since the reader will be someone who just got frustrated and possibly confused by a failed install).
Not blocking, up to you if you want to give it another pass to sharpen the formulations (just let me know if not).

@fkiraly
Copy link
Collaborator

fkiraly commented Dec 31, 2022

Other question: I do sometimes run into trouble with numba on M1. Did you encounter that at all?

@dainelli98
Copy link
Contributor Author

Other question: I do sometimes run into trouble with numba on M1. Did you encounter that at all?

To be honest, it has not been that long that I bought my current laptop. As of now, I have not experienced any issue, but that could be due to the fact that I have yet to use a package that leverages numba. Anyway, this also interests me, so I will check. If you want, I can get back to you with what I get.

@dainelli98
Copy link
Contributor Author

Excellent, thanks! I don't think the troubleshooting paragraph is too long, it is clear and helpful.

No blocking comments from my side.

I left some comments on how you could improve formulations - in tech writing and espceially tech instructions, it's worth being very clear and precise (since the reader will be someone who just got frustrated and possibly confused by a failed install). Not blocking, up to you if you want to give it another pass to sharpen the formulations (just let me know if not).

You are right, thanks for the advice. I will make the change, as I do not see any reason to not be as precise as possible.

Sharpen formulation so that the issue description and the workaround are more clear and tight to the actual actions that need to take place in order to avoid the issue.
@dainelli98
Copy link
Contributor Author

dainelli98 commented Dec 31, 2022

On the numba issue. As you may know already, Python packages can be distributed as wheels or source distribution. The first comes ready to be used, but the second needs to be built, which can cause issues on an ARM-based processor (The same issue occurs with prophet do to its dependency to pystan, that comes as source distribution).

In the case of numba, the proposed solution is to install it using conda:

conda install numba

Another solution seems to be to install with pip ta version that has a wheel, that is distributed as wheel:

pip install numba==0.55.2

It seems that as of now the last version is provided as a wheel, but this was done lately. So you may have experienced the issue when it was not like this.

It is weird that they sometimes do not provide a wheel for the last versions, but both workarounds seem to work on my mac.

@fkiraly
Copy link
Collaborator

fkiraly commented Dec 31, 2022

It is weird that they sometimes do not provide a wheel for the last versions, but both workarounds seem to work on my mac.

Is that worth adding, too?

@fkiraly
Copy link
Collaborator

fkiraly commented Dec 31, 2022

Thanks for ths tipps btw! At work, we have a mix of ARM and non-ARM macs, and the two behave differently...

@fkiraly
Copy link
Collaborator

fkiraly commented Dec 31, 2022

but that could be due to the fact that I have yet to use a package that leverages numba

sktime has numba as a core dependency, but it is used only in a couple estimators (not in the framework).

Add workaround information on how to avoid issue related to installing packages distributed as source distributions on devices using ARM-based processors.
@dainelli98
Copy link
Contributor Author

It is weird that they sometimes do not provide a wheel for the last versions, but both workarounds seem to work on my mac.

Is that worth adding, too?

The only reason to add it would be because the issue that the last version of numba that now is fixed could happen in the future with another package if they do a new release without providing the wheel.

As of now, all the packages not mentioned in the workaround do not present this issue, so at this moment this problem should not arise. If you think this is a bit overkill, it is easy to undo the commits.

@fkiraly
Copy link
Collaborator

fkiraly commented Dec 31, 2022

ok, makes sense. Let me know when you think you're done, for review and potential merge.

@dainelli98
Copy link
Contributor Author

dainelli98 commented Dec 31, 2022

ok, makes sense. Let me know when you think you're done, for review and potential merge.

For me it is ready for review as it is.

Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, great contribution!

Hope this will be helpful to all the poor souls with ARM macs out there.

@fkiraly
Copy link
Collaborator

fkiraly commented Dec 31, 2022

failures unrelated to this PR, see #4033

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Tensorflow failing on macOS
2 participants