submodlib
submodlib copied to clipboard
General C++ Compilation Issues
While the pip
installation works for Google Colab, I am still running into issues installing on a Windows system with C++20 VS build tools. There are quite a few warnings; more importantly, there are a couple errors that I've identified from the full log output (PIP Log Output.txt):
C:\Users\nbeck\AppData\Local\Temp\pip-install-d83pcjkv\submodlib_df4056be6b38418fbc6a76e7ace5d2d4\cpp\SetFunction.cpp(12) : error C4716: 'SetFunction::getEffectiveGroundSet': must return a value C:\Users\nbeck\AppData\Local\Temp\pip-install-d83pcjkv\submodlib_df4056be6b38418fbc6a76e7ace5d2d4\cpp\SetFunction.cpp(10) : error C4716: 'SetFunction::marginalGainWithMemoization': must return a value C:\Users\nbeck\AppData\Local\Temp\pip-install-d83pcjkv\submodlib_df4056be6b38418fbc6a76e7ace5d2d4\cpp\SetFunction.cpp(9) : error C4716: 'SetFunction::marginalGain': must return a value C:\Users\nbeck\AppData\Local\Temp\pip-install-d83pcjkv\submodlib_df4056be6b38418fbc6a76e7ace5d2d4\cpp\SetFunction.cpp(8) : error C4716: 'SetFunction::evaluateWithMemoization': must return a value C:\Users\nbeck\AppData\Local\Temp\pip-install-d83pcjkv\submodlib_df4056be6b38418fbc6a76e7ace5d2d4\cpp\SetFunction.cpp(7) : error C4716: 'SetFunction::evaluate': must return a value C:\Users\nbeck\AppData\Local\Temp\pip-install-d83pcjkv\submodlib_df4056be6b38418fbc6a76e7ace5d2d4\cpp\optimizers\LazierThanLazyGreedyOptimizer.cpp(49) : error C4716: 'printSortedSet': must return a value C:\Users\nbeck\AppData\Local\Temp\pip-install-d83pcjkv\submodlib_df4056be6b38418fbc6a76e7ace5d2d4\cpp\SetFunction.cpp(13) : error C4716: 'SetFunction::maximize': must return a value
All the errors revolve around the lack of a return statement in functions that have return types. As a suggestion, perhaps the SetFunction class could instead declare its functions as pure virtual functions so that they can be abstract definitions/declarations (as they do not do anything by their current definition). Furthermore, perhaps the SetFunction class could declare a blank virtual destructor as not all compilers will bind delete calls from base-class pointers to the most-derived class's destructor without this definition.
Sure. Can you pls go ahead and make the changes? Make sure to run all the tests after making the changes. After that you can create a pull-request so that I can review the changes and merge.
Sure, I can make these changes. I'll start on a new branch.