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

Python: Booster.load_model() should not require buffer writeability #3013

Closed
aldanor opened this issue Jan 6, 2018 · 4 comments
Closed

Python: Booster.load_model() should not require buffer writeability #3013

aldanor opened this issue Jan 6, 2018 · 4 comments

Comments

@aldanor
Copy link
Contributor

aldanor commented Jan 6, 2018

It is impossible to load a model from a buffer (e.g. bytestring) in Python, due to a bug where ctypes is used incorrectly to get a const char* pointer:

>>> bst.load_model(b'123')
TypeError: underlying buffer is not writable

Here's a short example of what it is basically trying to do:

>>> b = b'123'
>>> (ctypes.c_char * 3).from_buffer(buf)
TypeError: underlying buffer is not writable
@aldanor
Copy link
Contributor Author

aldanor commented Jan 6, 2018

See also: #542

@aldanor
Copy link
Contributor Author

aldanor commented Jan 6, 2018

Basically, instead of using .from_buffer(), you should use .from_buffer_copy() which only requires readability.

@aldanor
Copy link
Contributor Author

aldanor commented Jan 6, 2018

However, apparently, there is some problem with XGBoosterLoadModelFromBuffer() as it segfaults if you provide it with the dump buffer.

Here's a modified version that uses read-only buffer:

import ctypes
import xgboost
import xgboost.core

def xgb_load_model(buf):
    if isinstance(buf, str):
        buf = buf.encode()
    bst = xgboost.core.Booster()
    n = len(buf)
    length = xgboost.core.c_bst_ulong(n)
    ptr = (ctypes.c_char * n).from_buffer_copy(buf)
    xgboost.core._check_call(
        xgboost.core._LIB.XGBoosterLoadModelFromBuffer(bst.handle, ptr, length)
    )  # segfault
    return bst

@tqchen tqchen closed this as completed Jul 4, 2018
@hcho3 hcho3 mentioned this issue Jul 5, 2018
32 tasks
@hcho3
Copy link
Collaborator

hcho3 commented Jul 5, 2018

Consolidating to #3439. This issue should be re-opened if you or others decide to actively work on implementing this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants