Skip to content
This repository has been archived by the owner on Nov 21, 2023. It is now read-only.

Python 3 compatibility #110

Closed
wants to merge 17 commits into from
Closed

Conversation

jasonbunk
Copy link

This leaves the code as Python 2, but sets up the ability to use 2to3 for porting to Python 3 by fixing issues that 2to3 can't fix by itself. The messiest parts are in lib/core/config.py and the differences between data types bytes, str, and unicode. Notes on the porting process are recorded here: PYTHON2_TO_PYTHON3_NOTES.md. Tests in tests/ should pass for both Python 2.7 and 3.5, as well as tools/infer_simple.py.

@Beitadoge
Copy link

@jasonbunk Thanks, it is cool and useful for me ,but i have some questions, Should i install caffe2 with python3?Thank you for your reply

@jasonbunk
Copy link
Author

jasonbunk commented Feb 9, 2018

Yep. Just made this gist to build the latest caffe2 from source, in a virtual environment, assuming OpenCV 3.2+ is already installed. It doesn't need sudo privileges, unless you need to install the caffe2 prerequisites (build-essential, git, etc.).

https://gist.github.com/jasonbunk/9d61a49cc8402f32b1dabcf7b85bdcc6

Not sure if it's exporting $PYTHONPATH correctly, but it's how I got it to work again for now. It looks like the caffe2 devs are moving things around. A few days ago I had it pointing to caffe2/build_release/, where the folder "caffe2" would be installed, but that seems to have been moved to caffe2/build_release/lib/python3.5/site-packages/.

@DingkunLiu
Copy link

I meet another error when use this PR and use 2to3 to run detectron under python3. the "import Queue" in lib/utils/coordinator.py would be refactor as "import queue", which is duplicate with the variable name in functions.

@Erotemic
Copy link

Erotemic commented Mar 4, 2018

Instead of using 2to3 to convert the code to python3, why not just write code that is 2 and 3 compatible with the six library? Then there would be no need to run the 2to3 conversion script, which can be buggy and miss things.

@jasonbunk
Copy link
Author

I started this pull request to identify some of the minimal changes holding back using Detectron with Python 3. There are plenty of small things, like renaming cPickle to pickle, that would significantly increase the number of lines needing review, but aren't very important (they can be trivially fixed with find/replace, sed, 2to3, etc). In the long term, it would be good to move to something like six; but that would require more changes, causing this fork to drift further from the official master branch, and I wanted this to be easier to keep in sync by keeping its changes to a minimum.

@Erotemic
Copy link

Erotemic commented Mar 5, 2018

That's fair, this is definitely an easier and self-contained merge (which I hope will be accepted soon).

Also, note that I'd recommend not renaming cPickle to pickle. Instead, I'd replace import cPickle as pickle with from six.moves import cPickle as pickle', which will have the original behavior on Python2 but work just like import pickle` on Python3. Perhaps changes like that would be better suited for another PR.

@groot-1313
Copy link

groot-1313 commented Apr 9, 2018

I am using python 3, and have made the changes you mentioned. When I run the command,

python tools/infer_simple.py --cfg configs/12_2017_baselines/e2e_mas k_rcnn_R-101-FPN_2x.yaml --output-dir /tmp/detectron-visualizations --image-ext jpg --wts https://s3-us-west-2.amazonaws.com/detectron/35861858/12_2017_baselines/e2e_mask_rcnn_R-101-FPN_2x.yaml.02_32_51.SgT4y1cO/output/train/coco_2014_train:coco_2014_valminusminival/generalized_rcnn/model_final.pkl demo

Traceback (most recent call last):
File "tools/infer_simple.py", line 148, in
main(args)
File "tools/infer_simple.py", line 100, in main
model = infer_engine.initialize_model_from_cfg(args.weights)
File "/home/caffe2/build/cocoapi/PythonAPI/Detectron/lib/core/test_engine.py", line 328, in initialize_model_from_cfg
model = model_builder.create(cfg.MODEL.TYPE, train=False, gpu_id=gpu_id)
File "/home/caffe2/build/cocoapi/PythonAPI/Detectron/lib/modeling/model_builder.py", line 124, in create
return get_func(model_type_func)(model)
File "/home/caffe2/build/cocoapi/PythonAPI/Detectron/lib/modeling/model_builder.py", line 89, in generalized_rcnn
freeze_conv_body=cfg.TRAIN.FREEZE_CONV_BODY
File "/home/caffe2/build/cocoapi/PythonAPI/Detectron/lib/modeling/model_builder.py", line 229, in build_generic_detection_model
optim.build_data_parallel_model(model, _single_gpu_build_func)
File "/home/caffe2/build/cocoapi/PythonAPI/Detectron/lib/modeling/optimizer.py", line 54, in build_data_parallel_model
single_gpu_build_func(model)
File "/home/caffe2/build/cocoapi/PythonAPI/Detectron/lib/modeling/model_builder.py", line 169, in _single_gpu_build_func
blob_conv, dim_conv, spatial_scale_conv = add_conv_body_func(model)
File "/home/caffe2/build/cocoapi/PythonAPI/Detectron/lib/modeling/FPN.py", line 62, in add_fpn_ResNet101_conv5_body
model, ResNet.add_ResNet101_conv5_body, fpn_level_info_ResNet101_conv5
File "/home/caffe2/build/cocoapi/PythonAPI/Detectron/lib/modeling/FPN.py", line 103, in add_fpn_onto_conv_body
conv_body_func(model)
File "/home/caffe2/build/cocoapi/PythonAPI/Detectron/lib/modeling/ResNet.py", line 46, in add_ResNet101_conv5_body
return add_ResNet_convX_body(model, (3, 4, 23, 3))
File "/home/caffe2/build/cocoapi/PythonAPI/Detectron/lib/modeling/ResNet.py", line 101, in add_ResNet_convX_body
s, dim_in = add_stage(model, 'res2', p, n1, dim_in, 256, dim_bottleneck, 1)
File "/home/caffe2/build/cocoapi/PythonAPI/Detectron/lib/modeling/ResNet.py", line 83, in add_stage
inplace_sum=i < n - 1
File "/home/caffe2/build/cocoapi/PythonAPI/Detectron/lib/modeling/ResNet.py", line 172, in add_residual_block
tr = globals()[cfg.RESNETS.TRANS_FUNC](
KeyError: b'bottleneck_transformation'

@gadcam
Copy link
Contributor

gadcam commented May 31, 2018

@ir413 & @rbgirshick Do you think that if this PR is rebased & modified according to the current master you will consider to merge it ? There are 11 upvotes on it so it seems there is great support from the community to bring Python 3 support directly in the official repo.

@rbgirshick
Copy link
Contributor

@gadcam we look forward to supporting Python 3, but until recently were blocked by a boring internal issue on our end. Updating this PR may help speed things up, but I cannot guarantee that this exact PR will be merged. Given the broad set of changes, we may incorporate some of the edits here as well as make other changes that we decide are necessary.

@tengerye
Copy link

tengerye commented Aug 7, 2018

@rbgirshick Hi, any progress on this issue please?

@gadcam
Copy link
Contributor

gadcam commented Aug 11, 2018

@rbgirshick Why not merging this one ?
There is no merge conflict at the time I write.
It works almost out of the box.
The only problem I encountered in a few months (no training however) is https://github.com/facebookresearch/Detectron/pull/110/files#diff-5b9c584a1046a0afef241fd7b2cb9779R150
where you need to change

 md5sum = urllib.request.urlopen(url_md5sum).read().strip()

to

 md5sum = urllib.request.urlopen(url_md5sum).read().strip().decode('utf-8')

@jasonbunk could you include this fix in your awesome PR ?

@rbgirshick
Copy link
Contributor

It needs significant testing and we just haven't had time for that. Sorry for the slow progress and thanks for your patience. @ir413 is planning to look into py3 support, so hopefully we will resolve this soon.

@JaosonMa
Copy link

so ,hava someone build detectron(https://github.com/jasonbunk/Detectron) and caffe2 with python3 sucess? and works well?
i have train my models with python2,but on our server ,the python version is 3.0, so i have to update python2 to python3?
so does this works well ?

@gadcam
Copy link
Contributor

gadcam commented Aug 14, 2018

@JaosonMa I used this patch only for inference and it works well!
If you want to test it for training and give some feedback it would be great : if you ran into any bug it would be the chance to patch them with the help of the community :)

@JaosonMa
Copy link

JaosonMa commented Aug 16, 2018

@gadcam @jasonbunk
after build caffe2 and detectron with python3,
run test_to_pass.h
when test test_loader.py , error come out:
jasonma@img-server:~/detectron_python3$ python3 detectron/tests/test_loader.py
E0816 13:15:33.991026 14151 init_intrinsics_check.cc:43] CPU feature avx is present on your machine, but the Caffe2 binary is not compiled with it. It means you may not get the full speed of your CPU.
E0816 13:15:33.991058 14151 init_intrinsics_check.cc:43] CPU feature avx2 is present on your machine, but the Caffe2 binary is not compiled with it. It means you may not get the full speed of your CPU.
E0816 13:15:33.991065 14151 init_intrinsics_check.cc:43] CPU feature fma is present on your machine, but the Caffe2 binary is not compiled with it. It means you may not get the full speed of your CPU.
INFO test_loader.py: 56: Protobuf:
name: "dequeue_net_train"
op {
input: "gpu_0/roi_blobs_queue_ff558cf8-dc54-4939-b3a0-fad9b185edca"
output: "gpu_0/data"
name: ""
type: "DequeueBlobs"
device_option {
device_type: 1
cuda_gpu_id: 0
}
}
op {
input: "gpu_1/roi_blobs_queue_ff558cf8-dc54-4939-b3a0-fad9b185edca"
output: "gpu_1/data"
name: ""
type: "DequeueBlobs"
device_option {
device_type: 1
cuda_gpu_id: 1
}
}
external_input: "gpu_0/roi_blobs_queue_ff558cf8-dc54-4939-b3a0-fad9b185edca"
external_input: "gpu_1/roi_blobs_queue_ff558cf8-dc54-4939-b3a0-fad9b185edca"

terminate called after throwing an instance of 'caffe2::EnforceNotMet'
what(): [enforce fail at common_gpu.cc:129] error == cudaSuccess. 10 vs 0. Error at: /home/jasonma/pytorch/caffe2/core/common_gpu.cc:129: invalid device ordinal
*** Aborted at 1534396534 (unix time) try "date -d @1534396534" if you are using GNU date ***
PC: @ 0x7fb6fcd7e428 (unknown)
*** SIGABRT (@0x3ec00003747) received by PID 14151 (TID 0x7fb6ba7fc700) from PID 14151; stack trace: ***
@ 0x7fb6fd124390 (unknown)
@ 0x7fb6fcd7e428 (unknown)
@ 0x7fb6fcd8002a (unknown)
@ 0x7fb6e983e84d (unknown)
@ 0x7fb6e983c6b6 (unknown)
@ 0x7fb6e983b6a9 (unknown)
@ 0x7fb6e983c005 (unknown)
@ 0x7fb6e95a8f83 (unknown)
@ 0x7fb6e95a92eb (unknown)
@ 0x7fb6e983c90c (unknown)
@ 0x7fb6e60ef564 caffe2::CaffeCudaSetDevice()
@ 0x7fb6f8ada555 caffe2::ThreadLocalCUDAObjects::GetStream()
@ 0x7fb6f8ada9de caffe2::CUDAContext::FinishDeviceComputation()
@ 0x7fb6f8adaaf5 caffe2::CUDAContext::~CUDAContext()
@ 0x7fb6f8add304 caffe2::python::TensorFeeder<>::FeedTensor()
@ 0x7fb6f8a7c86e ZZN8pybind1112cpp_function10initializeIZN6caffe26python16addGlobalMethodsERNS_6moduleEEUlRKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEENS_6objectESE_E38_bJSD_SE_SE_EJNS_4nameENS_5scopeENS_7siblingEA1_cNS_3argESK_NS_5arg_vEEEEvOT_PFT0_DpT1_EDpRKT2_ENUlRNS_6detail13function_callEE1_4_FUNESZ
@ 0x7fb6f8ab26ce pybind11::cpp_function::dispatcher()
@ 0x4e9ba7 (unknown)
@ 0x5372f4 (unknown)
@ 0x540199 (unknown)
@ 0x53bd92 (unknown)
@ 0x5406df (unknown)
@ 0x53c1d0 (unknown)
@ 0x540f9b (unknown)
@ 0x4ebe37 (unknown)
@ 0x5c1797 (unknown)
@ 0x53920b (unknown)
@ 0x53b7e4 (unknown)
@ 0x53b7e4 (unknown)
@ 0x540f9b (unknown)
@ 0x4ebd23 (unknown)
@ 0x5c1797 (unknown)
Aborted (core dumped)

forgot this Q:
i found that
if name == 'main':
workspace.GlobalInit(['caffe2', '--caffe2_log_level=0'])
logger = logging_utils.setup_logging(name)
logger.setLevel(logging.DEBUG)
logging.getLogger('detectron.roi_data.loader').setLevel(logging.INFO)
np.random.seed(cfg.RNG_SEED)
cfg.TRAIN.ASPECT_GROUPING = False
cfg.NUM_GPUS = 2
assert_and_infer_cfg()
unittest.main()

it need 2 gpus ,but i only hava one ,so after change it , the test passed!

@JaosonMa
Copy link

finally , it sucess!!!
thank you so much !! @jasonbunk
home/jasonma/detectron_python3/image_test/des_img/46391_46391_1531292836528_E179197F.jpg
INFO infer_simple.py: 369: 6:Processing /home/jasonma/detectron_python3/image_test/src_img/46391_46391_1531292875579_2FE818A0.jpg
INFO infer_simple.py: 377: Inference time: 0.895s
INFO infer_simple.py: 379: | misc_bbox: 0.012s
INFO infer_simple.py: 379: | im_detect_bbox: 0.882s
.................................................................................................

@rbgirshick
Copy link
Contributor

Python 3 support required a significant amount of testing and finding corner cases involving serializing json and pickle files. We've landed support in commits 101-104.

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

Successfully merging this pull request may close these issues.

None yet

10 participants