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

Clean up transducer build #1159

Merged
merged 2 commits into from
Jan 9, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 24 additions & 15 deletions build_tools/setup_helpers/extension.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,29 +61,31 @@ def _get_ela(debug):


def _get_srcs():
return [str(p) for p in _CSRC_DIR.glob('**/*.cpp')]
srcs = [_CSRC_DIR / 'pybind.cpp']
srcs += list(_CSRC_DIR.glob('sox/**/*.cpp'))
if _BUILD_TRANSDUCER:
srcs += [_CSRC_DIR / 'transducer.cpp']
return [str(path) for path in srcs]


def _get_include_dirs():
dirs = [
str(_ROOT_DIR),
]
if _BUILD_SOX:
if _BUILD_SOX or _BUILD_TRANSDUCER:
dirs.append(str(_TP_INSTALL_DIR / 'include'))
if _BUILD_TRANSDUCER:
dirs.append(str(_TP_BASE_DIR / 'transducer' / 'submodule' / 'include'))
return dirs


def _get_extra_objects():
objs = []
libs = []
if _BUILD_SOX:
# NOTE: The order of the library listed bellow matters.
#
# (the most important thing is that dependencies come after a library
# e.g., sox comes first, flac/vorbis comes before ogg, and
# vorbisenc/vorbisfile comes before vorbis
libs = [
libs += [
'libsox.a',
'libmad.a',
'libFLAC.a',
Expand All @@ -97,27 +99,34 @@ def _get_extra_objects():
'libopencore-amrnb.a',
'libopencore-amrwb.a',
]
for lib in libs:
objs.append(str(_TP_INSTALL_DIR / 'lib' / lib))
if _BUILD_TRANSDUCER:
objs.append(str(_TP_BASE_DIR / 'build' / 'transducer' / 'libwarprnnt.a'))
return objs
libs += ['libwarprnnt.a']

return [str(_TP_INSTALL_DIR / 'lib' / lib) for lib in libs]


def _get_libraries():
return [] if _BUILD_SOX else ['sox']


def _build_third_party():
build_dir = str(_TP_BASE_DIR / 'build')
def _build_third_party(base_build_dir):
build_dir = os.path.join(base_build_dir, 'third_party')
os.makedirs(build_dir, exist_ok=True)
subprocess.run(
args=['cmake', '..'],
args=[
'cmake',
f'-DCMAKE_INSTALL_PREFIX={_TP_INSTALL_DIR}',
f'-DBUILD_SOX={"ON" if _BUILD_SOX else "OFF"}',
f'-DBUILD_TRANSDUCER={"ON" if _BUILD_TRANSDUCER else "OFF"}',
f'{_TP_BASE_DIR}'],
cwd=build_dir,
check=True,
)
command = ['cmake', '--build', '.']
if _BUILD_TRANSDUCER:
command += ['--target', 'install']
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

instrall target is only defined when building transducer.

subprocess.run(
args=['cmake', '--build', '.'],
args=command,
cwd=build_dir,
check=True,
)
Expand Down Expand Up @@ -145,5 +154,5 @@ def get_ext_modules(debug=False):
class BuildExtension(TorchBuildExtension):
def build_extension(self, ext):
if ext.name == _EXT_NAME and _BUILD_SOX:
_build_third_party()
_build_third_party(self.build_temp)
super().build_extension(ext)
1 change: 0 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ def run(self):
# Remove build directory
build_dirs = [
ROOT_DIR / 'build',
ROOT_DIR / 'third_party' / 'build',
]
for path in build_dirs:
if path.exists():
Expand Down
15 changes: 12 additions & 3 deletions third_party/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
cmake_minimum_required(VERSION 3.1)
cmake_minimum_required(VERSION 3.5)

project(torchaudio_third_parties)
include(ExternalProject)

set(INSTALL_DIR ${CMAKE_CURRENT_SOURCE_DIR}/install)
option(BUILD_SOX "Build libsox statically")
option(BUILD_TRANSDUCER "Build transducer")
mthrok marked this conversation as resolved.
Show resolved Hide resolved

SET(CMAKE_POSITION_INDEPENDENT_CODE ON)
mthrok marked this conversation as resolved.
Show resolved Hide resolved

if (BUILD_SOX)
include(ExternalProject)
set(INSTALL_DIR ${CMAKE_INSTALL_PREFIX})
set(ARCHIVE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/archives)
set(COMMON_ARGS --quiet --disable-shared --enable-static --prefix=${INSTALL_DIR} --with-pic --disable-dependency-tracking --disable-debug --disable-examples --disable-doc)

Expand Down Expand Up @@ -88,5 +94,8 @@ ExternalProject_Add(libsox
# See https://github.com/pytorch/audio/pull/1026
CONFIGURE_COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/build_codec_helper.sh ${CMAKE_CURRENT_SOURCE_DIR}/src/libsox/configure ${COMMON_ARGS} --with-lame --with-flac --with-mad --with-oggvorbis --without-alsa --without-coreaudio --without-png --without-oss --without-sndfile --with-opus --with-amrwb --with-amrnb --disable-openmp --without-sndio --without-pulseaudio
)
endif()

if(BUILD_TRANSDUCER)
add_subdirectory(transducer)
endif()
25 changes: 7 additions & 18 deletions third_party/transducer/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,20 +1,7 @@
CMAKE_MINIMUM_REQUIRED(VERSION 3.5)

PROJECT(rnnt_release)

SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O2")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removing this as I do not see this reflected in build logs. -O3 is used.


IF(APPLE)
ADD_DEFINITIONS(-DAPPLE)
ENDIF()

INCLUDE_DIRECTORIES(submodule/include)

SET(CMAKE_POSITION_INDEPENDENT_CODE ON)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

PIC argument has to be consistent across all the libraries. The current configuration works without this explicit line, but I moved this to the top level CMakeLitst.txt


ADD_DEFINITIONS(-DRNNT_DISABLE_OMP)

IF(APPLE)
ADD_DEFINITIONS(-DAPPLE)
EXEC_PROGRAM(uname ARGS -v OUTPUT_VARIABLE DARWIN_VERSION)
STRING(REGEX MATCH "[0-9]+" DARWIN_VERSION ${DARWIN_VERSION})
MESSAGE(STATUS "DARWIN_VERSION=${DARWIN_VERSION}")
Expand All @@ -30,9 +17,11 @@ ELSE()
ENDIF()

ADD_LIBRARY(warprnnt STATIC submodule/src/rnnt_entrypoint.cpp)
target_include_directories(warprnnt PUBLIC submodule/include)
set_target_properties(warprnnt PROPERTIES PUBLIC_HEADER submodule/include/rnnt.h)
Comment on lines +20 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: the commands are capitablized in this file :)

Suggested change
target_include_directories(warprnnt PUBLIC submodule/include)
set_target_properties(warprnnt PROPERTIES PUBLIC_HEADER submodule/include/rnnt.h)
TARGET_INCLUDE_DIRECTORIES(warprnnt PUBLIC submodule/include)
SET_TARGET_PROPERTIES(warprnnt PROPERTIES PUBLIC_HEADER submodule/include/rnnt.h)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is intended. only ancient CMake should use all capitals. I could have lowercased others but I left them untouched.

https://stackoverflow.com/a/32783814


INSTALL(TARGETS warprnnt
LIBRARY DESTINATION "lib"
ARCHIVE DESTINATION "lib")

INSTALL(FILES submodule/include/rnnt.h DESTINATION "submodule/include")
INSTALL(
TARGETS warprnnt
ARCHIVE DESTINATION "lib"
PUBLIC_HEADER DESTINATION "include")
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#include <torchaudio/csrc/sox/io.h>
#include <torchaudio/csrc/sox/utils.h>

TORCH_LIBRARY(torchaudio, m) {
TORCH_LIBRARY_FRAGMENT(torchaudio, m) {
//////////////////////////////////////////////////////////////////////////////
// sox_utils.h
//////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -74,18 +74,4 @@ TORCH_LIBRARY(torchaudio, m) {
m.def(
"torchaudio::sox_effects_apply_effects_file",
&torchaudio::sox_effects::apply_effects_file);

//////////////////////////////////////////////////////////////////////////////
// transducer.cpp
//////////////////////////////////////////////////////////////////////////////
#ifdef BUILD_TRANSDUCER
m.def("rnnt_loss(Tensor acts,"
"Tensor labels,"
"Tensor input_lengths,"
"Tensor label_lengths,"
"Tensor costs,"
"Tensor grads,"
"int blank_label,"
"int num_threads) -> int");
#endif
}
19 changes: 15 additions & 4 deletions torchaudio/csrc/transducer.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#ifdef BUILD_TRANSDUCER

#include <iostream>
#include <numeric>
#include <string>
Expand All @@ -8,6 +6,8 @@
#include <torch/script.h>
#include "rnnt.h"

namespace {

int64_t cpu_rnnt_loss(torch::Tensor acts,
torch::Tensor labels,
torch::Tensor input_lengths,
Expand Down Expand Up @@ -75,8 +75,19 @@ int64_t cpu_rnnt_loss(torch::Tensor acts,
return -1;
}

} // namespace

TORCH_LIBRARY_IMPL(torchaudio, CPU, m) {
m.impl("rnnt_loss", &cpu_rnnt_loss);
m.impl("rnnt_loss", &cpu_rnnt_loss);
}

#endif
TORCH_LIBRARY_FRAGMENT(torchaudio, m) {
Copy link
Contributor

Choose a reason for hiding this comment

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

TORCH_LIBRARY_FRAGMENT is neat :)

m.def("rnnt_loss(Tensor acts,"
"Tensor labels,"
"Tensor input_lengths,"
"Tensor label_lengths,"
"Tensor costs,"
"Tensor grads,"
"int blank_label,"
"int num_threads) -> int");
}