Skip to content

Commit

Permalink
Fixing MSVC support (#6)
Browse files Browse the repository at this point in the history
* fixing msvc not compiling bug

* changed cosine score and added feature weights for cpu

* addi
ng windows to ci

* fixing  bug in ci.yml

* Update ci.yml

* fixed bug in cosine

* fixing windows ci

* debug

* debugging

* fixing loss

* fixing test

* removing coverage
  • Loading branch information
benja263 authored Jan 22, 2025
1 parent 8cacac7 commit e0e9182
Show file tree
Hide file tree
Showing 38 changed files with 658 additions and 604 deletions.
67 changes: 46 additions & 21 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:

strategy:
matrix:
os: [ubuntu-latest]
os: [ubuntu-latest, windows-latest]
python-version: ['3.10']

steps:
Expand All @@ -33,36 +33,61 @@ jobs:
if: matrix.os == 'ubuntu-latest'
run: |
sudo apt-get update
sudo apt-get install -y lcov g++ gcc
# sudo apt-get install -y lcov g++ gcc
sudo apt-get install -y g++ gcc
- name: Install dependencies on Windows
if: matrix.os == 'windows-latest'
run: |
python -m pip install --upgrade pip
- name: Set up MSVC environment
if: matrix.os == 'windows-latest'
run: |
& "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvars64.bat"
- name: Set environment variable for coverage
if: matrix.os == 'ubuntu-latest'
run: echo "COVERAGE=1" >> $GITHUB_ENV

- name: Install Python dependencies and build the project with coverage
run: |
python -m pip install --upgrade pip
python -m pip install coverage
# python -m pip install coverage
pip install .
- name: Run tests
run: |
coverage run --source=gbrl -m unittest discover tests
- name: Generate coverage report
- name: Run tests
run: |
coverage report
coverage xml
python -m unittest discover tests
# - name: Run tests with coverage
# if: matrix.os == 'ubuntu-latest'
# run: |
# coverage run --source=gbrl -m unittest discover tests

# - name: Run tests without coverage
# if: matrix.os == 'windows-latest'
# run: |
# python -m unittest discover tests

- name: Generate C++ coverage report
run: |
lcov --capture --directory . --output-file coverage.info
lcov --remove coverage.info '/usr/*' --output-file coverage.info
lcov --list coverage.info
# - name: Generate coverage report
# if: matrix.os == 'ubuntu-latest'
# run: |
# coverage report
# coverage xml

# - name: Generate C++ coverage report
# if: matrix.os == 'ubuntu-latest'
# run: |
# lcov --capture --directory . --output-file coverage.info
# lcov --remove coverage.info '/usr/*' --output-file coverage.info
# lcov --list coverage.info

- name: Upload coverage reports as artifacts
uses: actions/upload-artifact@v4
with:
name: coverage-reports
path: |
coverage.xml
coverage.info
# - name: Upload coverage reports as artifacts
# if: matrix.os == 'ubuntu-latest'
# uses: actions/upload-artifact@v4
# with:
# name: coverage-reports
# path: |
# coverage.xml
# coverage.info
22 changes: 21 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,19 @@ if (POLICY CMP0148)
cmake_policy(SET CMP0148 NEW)
endif()

if (MSVC)
if(MSVC_VERSION LESS 1910)
message(
FATAL_ERROR
"The compiler ${CMAKE_CXX_COMPILER} doesn't support required C++14 features. Please use a newer MSVC."
)
endif()
endif()

if (CMAKE_BUILD_TYPE STREQUAL "Debug")
add_definitions(-DDEBUG)
endif()

# Allow overriding Python paths
if (DEFINED PYTHON_EXECUTABLE)
set(Python3_EXECUTABLE ${PYTHON_EXECUTABLE} CACHE FILEPATH "Path to the Python executable")
Expand Down Expand Up @@ -173,7 +186,11 @@ if(APPLE)
find_package(OpenMP REQUIRED)
endif()
else()
find_package(OpenMP REQUIRED)
if (MSVC)
find_package(OpenMP REQUIRED COMPONENTS C CXX)
else()
find_package(OpenMP REQUIRED)
endif()
set(OpenMP_CXX_FLAGS ${OpenMP_CXX_FLAGS})
set(OpenMP_CXX_LIB_NAMES ${OpenMP_CXX_LIB_NAMES})
set(OpenMP_omp_LIBRARY ${OpenMP_omp_LIBRARY})
Expand All @@ -184,6 +201,9 @@ if (OpenMP_FOUND)
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OpenMP_C_FLAGS}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")
if (MSVC)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /openmp:experimental")
endif()
endif()

# Add subdirectories for cpp and cuda
Expand Down
2 changes: 1 addition & 1 deletion gbrl/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
# https://nvlabs.github.io/gbrl/license.html
#
##############################################################################
__version__ = "1.0.6"
__version__ = "1.0.7"

from .ac_gbrl import (ActorCritic, GaussianActor, ContinuousCritic,
DiscreteCritic, ParametricActor)
Expand Down
10 changes: 5 additions & 5 deletions gbrl/ac_gbrl.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def __init__(self,
self.value_grad = None

@classmethod
def load_model(cls, load_name: str) -> "ActorCritic":
def load_model(cls, load_name: str, device: str) -> "ActorCritic":
"""Loads GBRL model from a file
Args:
Expand All @@ -92,11 +92,11 @@ def load_model(cls, load_name: str) -> "ActorCritic":

instance = cls.__new__(cls)
if os.path.isfile(policy_file) and os.path.isfile(value_file):
instance._model = SeparateActorCriticWrapper.load(load_name)
instance._model = SeparateActorCriticWrapper.load(load_name, device)
instance.shared_tree_struct = False
instance.bias = instance._model.policy_model.get_bias()
else:
instance._model = SharedActorCriticWrapper.load(load_name)
instance._model = SharedActorCriticWrapper.load(load_name, device)
instance.shared_tree_struct = True
instance.bias = instance._model.get_bias()
instance.value_optimizer = instance._model.value_optimizer
Expand Down Expand Up @@ -314,7 +314,7 @@ def step(self, observations: Union[np.ndarray, th.Tensor], policy_grad_clip: flo
self.grad = policy_grad

@classmethod
def load_model(cls, load_name: str) -> "ParametricActor":
def load_model(cls, load_name: str, device: str) -> "ParametricActor":
"""Loads GBRL model from a file
Args:
Expand All @@ -324,7 +324,7 @@ def load_model(cls, load_name: str) -> "ParametricActor":
ParametricActor: loaded ActorCriticModel
"""
instance = cls.__new__(cls)
instance._model = GBTWrapper.load(load_name)
instance._model = GBTWrapper.load(load_name, device)
instance.bias = instance._model.get_bias()
instance.policy_optimizer = instance._model.optimizer
instance.output_dim = instance._model.output_dim
Expand Down
32 changes: 21 additions & 11 deletions gbrl/gbrl_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#
##############################################################################
import os
from typing import Dict, List, Union, Tuple
from typing import Dict, List, Union, Tuple, Optional, Any

import numpy as np
import torch as th
Expand Down Expand Up @@ -49,6 +49,7 @@ def __init__(self, output_dim: int, tree_struct: Dict, optimizer: Union[Dict, Li
assert np.all(feature_weights >= 0), "feature weights contains non-positive values"
self.feature_weights = feature_weights


def reset(self) -> None:
if self.cpp_model is not None:
lrs = self.cpp_model.get_scheduler_lrs()
Expand Down Expand Up @@ -129,14 +130,15 @@ def export(self, filename: str, modelname: str = None) -> None:
print(f"Caught an exception in GBRL: {e}")

@classmethod
def load(cls, filename: str) -> "GBTWrapper":
def load(cls, filename: str, device: str) -> "GBTWrapper":
filename = filename.rstrip('.')
if '.gbrl_model' not in filename:
filename += '.gbrl_model'
assert os.path.isfile(filename), "filename doesn't exist!"
try:
instance = cls.__new__(cls)
instance.cpp_model = GBRL_CPP.load(filename)
instance.set_device(device)
metadata = instance.cpp_model.get_metadata()
instance.tree_struct = {'max_depth': metadata['max_depth'],
'min_data_in_leaf': metadata['min_data_in_leaf'],
Expand All @@ -162,6 +164,7 @@ def load(cls, filename: str) -> "GBTWrapper":
instance.iteration = metadata['iteration']
instance.total_iterations = metadata['iteration']
instance.student_model = None
instance.device = instance.params['device']
return instance
except RuntimeError as e:
print(f"Caught an exception in GBRL: {e}")
Expand All @@ -186,12 +189,12 @@ def set_bias(self, bias: Union[np.ndarray, float]) -> None:
raise TypeError("Input should be a numpy array or float")

if isinstance(bias, float):
bias = np.array([float])
bias = np.ndarray([float])

if bias.ndim > 1:
bias = bias.ravel()
elif bias.ndim == 0:
bias = np.array([bias.item()]) # Converts 0D arrays to 1D
bias = np.ndarray([bias.item()]) # Converts 0D arrays to 1D
try:
self.cpp_model.set_bias(bias)
except RuntimeError as e:
Expand Down Expand Up @@ -246,7 +249,9 @@ def shap(self, features: Union[np.ndarray, th.Tensor]) -> np.ndarray:
base_poly, norm_values, offset = get_poly_vectors(self.params['max_depth'], numerical_dtype)
return self.cpp_model.ensemble_shap(num_features, cat_features, np.ascontiguousarray(norm_values), np.ascontiguousarray(base_poly), np.ascontiguousarray(offset))

def set_device(self, device: str) -> None:
def set_device(self, device: Union[str, th.device]) -> None:
if isinstance(device, th.device):
device = device.type
try:
self.cpp_model.to_device(device)
self.device = device
Expand Down Expand Up @@ -292,7 +297,7 @@ def distil(self, obs: Union[np.ndarray, th.Tensor], targets: np.ndarray, params:

bias = np.mean(targets, axis=0)
if isinstance(bias, float):
bias = np.array([bias])
bias = np.ndarray([bias])
self.student_model.set_bias(bias.astype(numerical_dtype))
tr_loss = self.student_model.fit(num_obs, cat_obs, targets, params['min_steps'])
while tr_loss> params.get('min_distillation_loss', 0.1):
Expand All @@ -308,6 +313,9 @@ def distil(self, obs: Union[np.ndarray, th.Tensor], targets: np.ndarray, params:
def copy(self):
return self.__copy__()

def print_ensemble_metadata(self):
self.cpp_model.print_ensemble_metadata()

def __copy__(self):
copy_ = GBTWrapper(self.output_dim, self.tree_struct.copy(), [opt.copy() if opt is not None else opt for opt in self.optimizer], self.gbrl_params, self.verbose, self.device)
copy_.iteration = self.iteration
Expand Down Expand Up @@ -413,10 +421,12 @@ def plot_tree(self, tree_idx: int, filename: str) -> None:
self.value_model.plot_tree(tree_idx, filename.rstrip(".") + "_value")

@classmethod
def load(cls, filename: str) -> "SeparateActorCriticWrapper":
def load(cls, filename: str, device: str) -> "SeparateActorCriticWrapper":
instance = cls.__new__(cls)
instance.policy_model = GBTWrapper.load(filename + '_policy')
instance.value_model = GBTWrapper.load(filename + '_value')
instance.policy_model = GBTWrapper.load(filename + '_policy', device)
instance.value_model = GBTWrapper.load(filename + '_value', device)
instance.policy_model.set_device(device)
instance.value_model.set_device(device)
instance.tree_struct = instance.policy_model.tree_struct
instance.total_iterations = instance.policy_model.iteration + instance.value_model.iteration
instance.output_dim = instance.policy_model.output_dim
Expand Down Expand Up @@ -527,8 +537,8 @@ def predict_critic(self, observations: Union[np.ndarray, th.Tensor], requires_gr
return pred_values

@classmethod
def load(cls, filename: str) -> "SharedActorCriticWrapper":
instance = super().load(filename)
def load(cls, filename: str, device: str) -> "SharedActorCriticWrapper":
instance = super().load(filename, device)
instance.policy_optimizer = instance.optimizer[0]
instance.value_optimizer = None if len(instance.optimizer) != 2 else instance.optimizer[1]
return instance
Expand Down
10 changes: 5 additions & 5 deletions gbrl/gbt.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
# https://nvlabs.github.io/gbrl/license.html
#
##############################################################################
from typing import Dict, List, Union, Tuple, Optional
from typing import Dict, List, Union, Tuple, Optional, Any

import numpy as np
import torch as th
Expand Down Expand Up @@ -115,7 +115,7 @@ def set_bias_from_targets(self, targets: Union[np.ndarray, th.Tensor]):
arr = arr[:, np.newaxis]
mean_arr = np.mean(arr, axis=0)
if isinstance(mean_arr, float):
mean_arr = np.array([mean_arr])
mean_arr = np.ndarray([mean_arr])
self._model.set_bias(mean_arr.astype(np.single)) # GBRL only works with float32

def get_iteration(self) -> int:
Expand Down Expand Up @@ -172,7 +172,7 @@ def get_params(self) -> Tuple[np.ndarray, np.ndarray]:
params = (params[0].detach().cpu().numpy(), params[1].detach().cpu().numpy())
return params, self.grad

def fit(self, X: Union[np.ndarray, th.Tensor], targets: Union[np.ndarray, th.Tensor], iterations: int, shuffle: bool=True, loss_type: str='MultiRMSE') -> float:
def fit(self, X: Union[np.ndarray, th.Tensor], targets: Union[np.ndarray, th.Tensor], iterations: int, shuffle: bool = True, loss_type: str = 'MultiRMSE') -> float:
"""Fit multiple iterations (as in supervised learning)
Args:
Expand Down Expand Up @@ -243,7 +243,7 @@ def export_model(self, filename: str, modelname: str = None) -> None:
self._model.export(filename, modelname)

@classmethod
def load_model(cls, load_name: str) -> "GBRL":
def load_model(cls, load_name: str, device: str) -> "GBRL":
"""Loads GBRL model from a file
Args:
Expand All @@ -253,7 +253,7 @@ def load_model(cls, load_name: str) -> "GBRL":
GBRL instance
"""
instance = cls.__new__(cls)
instance._model = GBTWrapper.load(load_name)
instance._model = GBTWrapper.load(load_name, device)
instance.optimizer = instance._model.optimizer
instance.output_dim = instance._model.output_dim
instance.verbose = instance._model.verbose
Expand Down
40 changes: 34 additions & 6 deletions gbrl/src/cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,47 @@ else()
message(STATUS "Did not Find Graphviz")
endif()


if (CMAKE_SYSTEM_NAME STREQUAL "Linux")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O3 -std=c++14 -Wall -Wpedantic -Wextra -march=native -fPIC")
set(BASE_CXX_FLAGS "-Wall -Wpedantic -Wextra -std=c++14 -fPIC")
elseif (CMAKE_SYSTEM_NAME STREQUAL "Darwin")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O3 -std=c++14 -Wall -Wpedantic -Wextra -fPIC")
set(BASE_CXX_FLAGS "-Wall -Wpedantic -Wextra -std=c++14 -fPIC")
elseif (WIN32)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /O2 /std:c++14 /W3")
set(BASE_CXX_FLAGS "/std:c++14 /W3")
endif()

if (MSVC)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /W4 /MP") # Enable warnings and multi-processor compilation
set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>DLL")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /permissive-")
endif()

# Debug-specific flags
if (CMAKE_BUILD_TYPE STREQUAL "Debug")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g")
message(STATUS "Configuring for Debug build.")
if (CMAKE_SYSTEM_NAME STREQUAL "Linux" OR CMAKE_SYSTEM_NAME STREQUAL "Darwin")
set(DEBUG_CXX_FLAGS "-g")
elseif (MSVC)
set(DEBUG_CXX_FLAGS "/Zi") # Debug symbols
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /DEBUG") # Generate .pdb
endif()
else()
message(STATUS "Configuring for Release build.")
if (CMAKE_SYSTEM_NAME STREQUAL "Linux")
set(RELEASE_CXX_FLAGS "-O3 -march=native")
elseif (CMAKE_SYSTEM_NAME STREQUAL "Darwin")
set(RELEASE_CXX_FLAGS "-O3")
elseif (WIN32)
if (MSVC)
set(RELEASE_CXX_FLAGS "/O2 /Ob2 /Oi /Ot /Oy")
else()
set(RELEASE_CXX_FLAGS "/O2")
endif()
endif()
endif()

# Combine base flags with build-specific flags
set(CMAKE_CXX_FLAGS "${BASE_CXX_FLAGS} ${CMAKE_CXX_FLAGS} ${DEBUG_CXX_FLAGS} ${RELEASE_CXX_FLAGS}")


add_library(gbrl_cpp_src OBJECT ${CPP_SOURCES})
# target_compile_definitions(gbrl_cpp_src PUBLIC MODULE_NAME="gbrl_cpp")

Loading

0 comments on commit e0e9182

Please sign in to comment.