Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

on windows, functions of libbleu are not found #292

Closed
hollyhook opened this issue Sep 26, 2018 · 5 comments
Closed

on windows, functions of libbleu are not found #292

hollyhook opened this issue Sep 26, 2018 · 5 comments
Labels

Comments

@hollyhook
Copy link

using microsoft visual compiler 14.0, fairseq 0.5.0
building libbleu dll by calling python setup.py build develop

functions in pyd are only exposed if they are declared as __declspec(dllexport).

problem is fixed if declaration of all exported C++ functions are changed, e.g.

extern "C" {
  __declspec(dllexport) void bleu_zero_init(bleu_stat* stat) {
    std::memset(stat, 0, sizeof(bleu_stat));
  }
}
@ShilinHe
Copy link

Have the same issue, can you share more details about the fix? where to put this code snippet?

@ShilinHe
Copy link

Let me elaborate more on the solution of @hollyhook. This issue is caused by the name changing problem during compiling. The compiler changed the function name, and thereby we cannot find the function.

To solve the problem, you should add __declspec(dllexport) before the function declaration. In this line (fairseq/fairseq/clib/libbleu/libbleu.cpp file), add the __declspec(dllexport) before function void bleu_zero_init, void bleu_one_init , and void bleu_add. Finally, call the python setup.py build develop, the problem would be fixed.

@huihuifan
Copy link
Contributor

Thanks @ShilinHe ! @hollyhook , hope that resolves your issue. Please re-open if not.

@GeorgeS2019
Copy link

@myleott this modification needs to be included in the next release as part of pip install fairseq for Windows.

@erip
Copy link
Contributor

erip commented Jan 5, 2020

I propose this should be re-opened. Requiring users to build your library from scratch is possible, but painful. I am not an expert at cross-compiling, but would it be as simple as conditionally adding the __declspec to the files in master? I don't know if that directive impacts Linux or OS X builds.

@myleott myleott reopened this Jan 6, 2020
facebook-github-bot pushed a commit that referenced this issue Jan 6, 2020
#1584)

Summary:
…s DLL exports.

# Before submitting

- [x] Was this discussed/approved via a Github issue? (no need for typos, doc improvements)
- [x] Did you read the [contributor guideline](https://github.com/pytorch/fairseq/blob/master/CONTRIBUTING.md)?
- [x] Did you make sure to update the docs?
- [x] Did you write any new necessary tests?

## What does this PR do?
Fixes #292

☝️ technically this issue should remain open

## PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

## Did you have fun?
👍
Pull Request resolved: #1584

Differential Revision: D19289792

Pulled By: myleott

fbshipit-source-id: 96af49e2ed808dde3f682906bc2d6e074b522c39
moussaKam pushed a commit to moussaKam/language-adaptive-pretraining that referenced this issue Sep 29, 2020
facebookresearch#1584)

Summary:
…s DLL exports.

# Before submitting

- [x] Was this discussed/approved via a Github issue? (no need for typos, doc improvements)
- [x] Did you read the [contributor guideline](https://github.com/pytorch/fairseq/blob/master/CONTRIBUTING.md)?
- [x] Did you make sure to update the docs?
- [x] Did you write any new necessary tests?

## What does this PR do?
Fixes facebookresearch#292

☝️ technically this issue should remain open

## PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

## Did you have fun?
👍
Pull Request resolved: facebookresearch#1584

Differential Revision: D19289792

Pulled By: myleott

fbshipit-source-id: 96af49e2ed808dde3f682906bc2d6e074b522c39
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants