-
Notifications
You must be signed in to change notification settings - Fork 99
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
When version mismatch happens, show versions #325
Conversation
Codecov Report
@@ Coverage Diff @@
## mainline #325 +/- ##
==============================================
- Coverage 85.25% 85.07% -0.18%
+ Complexity 44 42 -2
==============================================
Files 108 108
Lines 8360 8374 +14
Branches 48 40 -8
==============================================
- Hits 7127 7124 -3
- Misses 1209 1227 +18
+ Partials 24 23 -1
Continue to review full report at Codecov.
|
<< TREELITE_VER_MINOR << "." << TREELITE_VER_PATCH << std::endl | ||
<< "The model checkpoint was generated from Treelite version " << major_ver << "." | ||
<< minor_ver << "." << patch_ver; | ||
throw std::runtime_error(oss.str()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is perfect!
std::string& version_str = TreeliteAPIErrorStore::Get()->version_str; | ||
version_str = oss.str(); | ||
return version_str.c_str(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this does solve the case when the user doesn't have a model but wants to check a version (e.g. during automated deployment, or to double-check deployment results). However, it seems this requires to build a separate application, whose sole purpose is to check the treelite version?
I agree that it's a neat API for e.g. server startup-time checking of version. However, it seems to me the same can be achieved by
static const char[] TREELITE_VERSION = "TREELITE_VERSION_" #TREELITE_VER_MAJOR "." #TREELITE_VER_MINOR "." #TREELITE_VER_PATCH;
in
Line 10 in 2e41697
The reason I'm insisting on this way is that this allows to look from any linux shell as well as a C/C++ application (the application can strcmp with what it expects, output this into any stream/string/stdout).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@levsnv Please see my latest commit. You can now get the version number using strings
:
$ strings libtreelite.dylib -o -t x | grep VERSION
2948ed TREELITE_VERSION_2.1.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great! thanks for such a quick turnaround!
The 2.2.0 version of Treelite incorporates the following major improvements: * dmlc/treelite#314 * dmlc/treelite#322, dmlc/treelite#327 * dmlc/treelite#325 * dmlc/treelite#332 * dmlc/treelite#330 * dmlc/treelite#333 * dmlc/treelite#334 * dmlc/treelite#304 * dmlc/treelite#335 In particular, dmlc/treelite#332, dmlc/treelite#330, dmlc/treelite#333 are required for #4447. Requires rapidsai/integration#412. EDIT. Using 2.2.1 patch release, to incorporate a hotfix (dmlc/treelite#340). Authors: - Philip Hyunsu Cho (https://github.com/hcho3) Approvers: - AJ Schmidt (https://github.com/ajschmidt8) - Dante Gama Dessavre (https://github.com/dantegd) URL: #4484
The 2.2.0 version of Treelite incorporates the following major improvements: * dmlc/treelite#314 * dmlc/treelite#322, dmlc/treelite#327 * dmlc/treelite#325 * dmlc/treelite#332 * dmlc/treelite#330 * dmlc/treelite#333 * dmlc/treelite#334 * dmlc/treelite#304 * dmlc/treelite#335 In particular, dmlc/treelite#332, dmlc/treelite#330, dmlc/treelite#333 are required for rapidsai#4447. Requires rapidsai/integration#412. EDIT. Using 2.2.1 patch release, to incorporate a hotfix (dmlc/treelite#340). Authors: - Philip Hyunsu Cho (https://github.com/hcho3) Approvers: - AJ Schmidt (https://github.com/ajschmidt8) - Dante Gama Dessavre (https://github.com/dantegd) URL: rapidsai#4484
Before:
After:
Also, add the C API function
TreeliteQueryTreeliteVersion
so that users can query the version of the Treelite library.