YouTokenToMe icon indicating copy to clipboard operation
YouTokenToMe copied to clipboard

compilation issues

Open jwijffels opened this issue 4 years ago • 6 comments

I'm trying to merge the dropout functionality in the R package at https://github.com/bnosac/tokenizers.bpe Getting the following errors however when compiling.

C:/Rtools/mingw_32/bin/g++  -std=gnu++11 -I"C:/PROGRA~1/R/R-35~1.2/include" -DNDEBUG -pthread -DSTRICT_R_HEADERS -I./youtokentome/cpp -I. -I"C:/Users/Jan/Documents/R/win-library/3.5/Rcpp/include"        -O2 -Wall  -mtune=generic -c youtokentome/cpp/bpe.cpp -o youtokentome/cpp/bpe.o
youtokentome/cpp/bpe.cpp:1468:16: error: 'void vkcom::BasePriorityQueue<T>::push(T) [with T = vkcom::BaseEncoder::encode_sentence(const string&, const vkcom::EncodingConfig&, vkcom::OutputType) const::MergeEvent2]', declared using local type 'vkcom::BaseEncoder::encode_sentence(const string&, const vkcom::EncodingConfig&, vkcom::OutputType) const::MergeEvent2', is used but never defined [-fpermissive]
   virtual void push(T x) = 0;
                ^
youtokentome/cpp/bpe.cpp:1469:16: error: 'bool vkcom::BasePriorityQueue<T>::pop(T&) [with T = vkcom::BaseEncoder::encode_sentence(const string&, const vkcom::EncodingConfig&, vkcom::OutputType) const::MergeEvent2]', declared using local type 'vkcom::BaseEncoder::encode_sentence(const string&, const vkcom::EncodingConfig&, vkcom::OutputType) const::MergeEvent2', is used but never defined [-fpermissive]
   virtual bool pop(T& x) = 0;

jwijffels avatar Nov 20 '19 16:11 jwijffels

I changed BasePriorityQueue to make push and pop not pure virtual function by adding a default implementation to mitigate this issue. Still not sure why compilation fails.

template<typename T>
class BasePriorityQueue {
 public:
  virtual void push(T x) { std::cout << "call to pure virtual function BasePriorityQueue::push!" << std::endl; };
  virtual bool pop(T& x) { std::cout << "call to pure virtual function BasePriorityQueue::pop!" << std::endl; };
  virtual ~BasePriorityQueue() {}
};

I am using GCC 4.8.5 on Ubuntu.

felixhao28 avatar Jan 13 '20 03:01 felixhao28

If this solves the issue, can you make a pull request in the YouTokenToMe repository.

jwijffels avatar Jan 13 '20 08:01 jwijffels

No this change should not be made, because the original code without my patch compiles just fine on Windows. So I think the issue has something to do with compiler settings rather than the code itself. The real fix lies somewhere in the build settings.

felixhao28 avatar Jan 13 '20 10:01 felixhao28

I'm running Windows as well and the following did not compile on Windows as mentionned in the error message above C:/Rtools/mingw_32/bin/g++ -std=gnu++11 -I"C:/PROGRA~1/R/R-35~1.2/include" -DNDEBUG -pthread -DSTRICT_R_HEADERS -I./youtokentome/cpp -I. -I"C:/Users/Jan/Documents/R/win-library/3.5/Rcpp/include" -O2 -Wall -mtune=generic -c youtokentome/cpp/bpe.cpp -o youtokentome/cpp/bpe.o

jwijffels avatar Jan 13 '20 11:01 jwijffels

By saying Windows I meant Visual Studio.

felixhao28 avatar Jan 14 '20 07:01 felixhao28

I added "-fpermissive" to the compiler options in setup.py and got it to build. Not sure if that is really a solution though.

geert56 avatar Oct 08 '21 13:10 geert56