Skip to content

Fixing byte-order consistency/mismatch for cross-endian platform#1181

Merged
ogrisel merged 12 commits intojoblib:masterfrom
pradghos:s390x-cross-endian-fix
Jun 11, 2021
Merged

Fixing byte-order consistency/mismatch for cross-endian platform#1181
ogrisel merged 12 commits intojoblib:masterfrom
pradghos:s390x-cross-endian-fix

Conversation

@pradghos
Copy link
Copy Markdown
Contributor

@pradghos pradghos commented May 5, 2021

@pradghos pradghos mentioned this pull request May 5, 2021
Closed
@pradghos pradghos changed the title Fixing byte-order consistency/missmatch for cross-endian platform Fixing byte-order consistency/mismatch for cross-endian platform May 5, 2021
@pradghos
Copy link
Copy Markdown
Contributor Author

pradghos commented May 5, 2021 •
edited
Loading

Circle CI failed at ./continuous_integration/build_doc.sh

result = self._call_chain(self.handle_open, protocol, protocol +
File "/home/circleci/miniconda/envs/testenv/lib/python3.9/urllib/request.py", line 494, in _call_chain
  result = func(*args)
File "/home/circleci/miniconda/envs/testenv/lib/python3.9/urllib/request.py", line 1389, in https_open
  return self.do_open(http.client.HTTPSConnection, req,
File "/home/circleci/miniconda/envs/testenv/lib/python3.9/urllib/request.py", line 1349, in do_open
  raise URLError(err)
urllib.error.URLError: <urlopen error [Errno 110] Connection timed out>

Makefile:11: recipe for target 'html' failed

Before this commit https://github.com/joblib/joblib/pull/1181/commits/d9e5adbcc5478159310ebd58b3c070c5feea7e7d all were green except ci/circleci: build

Now joblib.joblib (Testing linux_py37_distributed) failed at test_nested_loop_error_in_grandchild_resource_tracker_silent which looks like unrelated to this PR.

Any pointers would really help . Thank you !

@pradghos
Copy link
Copy Markdown
Contributor Author

pradghos commented May 10, 2021

@ogrisel It would be great if you can take a look. Thanks in advance !

@lesteve
Copy link
Copy Markdown
Member

lesteve commented May 27, 2021

Thanks a lot for this PR! What would be great is to have a test for this.

Is there a way to store a pickle string obtained from a big endian machine and test that it can be loaded fine in the CI (little endian machine)?

I think we can ignore the CI failures as they do seem spurious.

@ogrisel
Copy link
Copy Markdown
Contributor

ogrisel commented May 27, 2021

Alternatively could you just please add a couple of unit tests for _is_numpy_array_byte_order_mismatch alone (independently of jolib.load / dump)?

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 8, 2021 •
edited
Loading

Codecov Report

Merging #1181 (f8a2f16) into master (754433f) will increase coverage by 0.02%.
The diff coverage is 94.73%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1181      +/-   ##
==========================================
+ Coverage   94.51%   94.54%   +0.02%     
==========================================
  Files          47       47              
  Lines        6997     7034      +37     
==========================================
+ Hits         6613     6650      +37     
  Misses        384      384
Impacted Files Coverage Δ
joblib/test/test_numpy_pickle.py 93.33% <92.59%> (-0.04%) ⬇️
joblib/numpy_pickle.py 98.01% <100.00%> (+<0.01%) ⬆️
joblib/numpy_pickle_compat.py 91.08% <100.00%> (+0.18%) ⬆️
joblib/numpy_pickle_utils.py 93.25% <100.00%> (+0.57%) ⬆️
joblib/test/test_store_backends.py 97.14% <0.00%> (+5.71%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 754433f...f8a2f16. Read the comment docs.

@pradghos
Copy link
Copy Markdown
Contributor Author

pradghos commented Jun 8, 2021

@lesteve @ogrisel Thank you very much for the review. I have added test. Please let me know in case of any further comments.

ogrisel
ogrisel reviewed Jun 11, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nitpick. I plan to push a new commit to better test the byteswap operation itself after that.

@ogrisel ogrisel merged commit 0fa2cb9 into joblib:master Jun 11, 2021
@ogrisel
Copy link
Copy Markdown
Contributor

ogrisel commented Jun 11, 2021

Merged!, thanks @pradghos!

@lesteve lesteve mentioned this pull request Oct 7, 2021
Closed
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 7, 2021
py-joblib: updated to 1.1.0
04c14ac 
Release 1.1.0
Fix byte order inconsistency issue during deserialization using joblib.load in cross-endian environment: the numpy arrays are now always loaded to use the system byte order, independently of the byte order of the system that serialized the pickle. joblib/joblib#1181
Fix joblib.Memory bug with the ignore parameter when the cached function is a decorated function. joblib/joblib#1165
Fix joblib.Memory to properly handle caching for functions defined interactively in a IPython session or in Jupyter notebook cell. joblib/joblib#1214
Update vendored loky (from version 2.9 to 3.0) and cloudpickle (from version 1.6 to 2.0) joblib/joblib#1218
jjerphan pushed a commit to jjerphan/joblib that referenced this pull request Oct 11, 2021
@pradghos @ogrisel @jjerphan
Fixing byte-order consistency/mismatch for cross-endian platform (job…
9e9c233 
…lib#1181)



Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 14, 2021
py-joblib: updated to 1.1.0
37df902 
Release 1.1.0
Fix byte order inconsistency issue during deserialization using joblib.load in cross-endian environment: the numpy arrays are now always loaded to use the system byte order, independently of the byte order of the system that serialized the pickle. joblib/joblib#1181
Fix joblib.Memory bug with the ignore parameter when the cached function is a decorated function. joblib/joblib#1165
Fix joblib.Memory to properly handle caching for functions defined interactively in a IPython session or in Jupyter notebook cell. joblib/joblib#1214
Update vendored loky (from version 2.9 to 3.0) and cloudpickle (from version 1.6 to 2.0) joblib/joblib#1218
This was referenced Sep 30, 2022
Closed
Closed
This was referenced Sep 30, 2022
Closed
Closed
@cmarmo cmarmo mentioned this pull request Oct 11, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ogrisel ogrisel ogrisel approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pradghos @lesteve @ogrisel