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

PyTorch Apple Silicon / macosx-arm64 support #1459

Closed
sbrunk opened this issue Jan 8, 2024 · 9 comments
Closed

PyTorch Apple Silicon / macosx-arm64 support #1459

sbrunk opened this issue Jan 8, 2024 · 9 comments

Comments

@sbrunk
Copy link
Contributor

sbrunk commented Jan 8, 2024

I recently got access to a Apple silicon machine, so I started to look into getting a native macosx-arm64 build for PyTorch. While we'll need to cross-build this in CI eventually, I've tried a native local build first so I can immediately run and test it.

I'm able to build cpython and libtorch with minimal changes. I ran into issues with OpenMP which I still need to figure out, so I've disabled it for now (I've seen that there's some special treatment of libomp in the native build).

One thing I'm currently struggling with is that the conversion from Vector to ArrayRef is missing in my local build, i.e.

pytorch/src/gen/java/org/bytedeco/pytorch/TensorIndexArrayRef.java

-  public TensorArrayRef(@ByRef TensorVector vec) { super((Pointer)null); allocate(vec); }
-  private native void allocate(@ByRef TensorVector vec);

Which causes all kinds of trouble as calling code now silently uses the Pointer constructor instead.

Could this be due to the newer version of clang?

clang --version
Apple clang version 15.0.0 (clang-1500.1.0.2.5)
Target: arm64-apple-darwin23.1.0

If I add these constructors manually back to the bindings, I get linking errors.

Any hints appreciated.

#1069

@HGuillemet
Copy link
Collaborator

Do you mean that the generated TensorArrayRef.java or TensorIndexArrayRef.java is different when building on a Linux or Windows and on MacOS ?
If yes, can you check if c10/util/ArrayRef.h differs ?

@sbrunk
Copy link
Contributor Author

sbrunk commented Jan 9, 2024

Yes I have ~15 *ArrayRef.java files that are different, all missing this constructor. I think doesn't happen in the MacOS CI build (currently only x86_64), because that one is working fine in tests.

I need to a diff later when I have access to the machine but I've checked already that the ArrayRef constructor is there.

  /// Construct an ArrayRef from a std::vector.
  // The enable_if stuff here makes sure that this isn't used for
  // std::vector<bool>, because ArrayRef can't work on a std::vector<bool>
  // bitfield.
  template <typename A>
  /* implicit */ ArrayRef(const std::vector<T, A>& Vec)
      : Data(Vec.data()), Length(Vec.size()) {
    static_assert(
        !std::is_same<T, bool>::value,
        "ArrayRef<bool> cannot be constructed from a std::vector<bool> bitfield.");
  }

Since I'm using the maven build from javacpp-presets and therefore pytorch/cppbuild.sh which checks out 2.1.2 I had assumed that the headers should be the same no matter which OS.

@HGuillemet
Copy link
Collaborator

Yes, that's quite weird.
Are you sure you're using the same versions of the presets ? PR #1455 introduce some changes about ArrayRef, but unrelated. You can try it out.
New commits will be pushed to this PR as soon as bytedeco/javacpp#733 is merged

@sbrunk
Copy link
Contributor Author

sbrunk commented Jan 9, 2024

I need to check the exact commit later but it should be current master. I'll also try #1455.

I also just ran a local build of cf80024 on a Linux machine, and I'm seeing the same behavior. So I suspect it might not be macos specific, but perhaps due to recent changes in the parser?

Could it perhaps be related to what @benshiffman has seen for templated operator overloads? bytedeco/javacpp#732 (comment)

Templated operator overloads with a std::vector as the right-hand parameter are not mapping.

@HGuillemet
Copy link
Collaborator

Perhaps.
But I do see the constructor in my linux build.
Could you try with the latest javacpp that @saudet merged some minutes ago ?

@sbrunk
Copy link
Contributor Author

sbrunk commented Jan 9, 2024

Still the same result.

cd javacpp
git pull
mvn install

Should be enough that javacpp-presets picks up the local javacpp shapshot right?

@HGuillemet
Copy link
Collaborator

RIght. And if you build now PR #1455 ?

@sbrunk
Copy link
Contributor Author

sbrunk commented Jan 9, 2024

Yes that's it. With #1455 the constructors are mapped again! Tested sucessfully on Linux and on macosx-arm64.

@saudet
Copy link
Member

saudet commented Jan 10, 2024

Duplicate of #1069

@saudet saudet marked this as a duplicate of #1069 Jan 10, 2024
@saudet saudet closed this as completed Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants