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

Updated typetools to allow java 9,10,11 #6819

Conversation

Projects
None yet
2 participants
@smagellan
Copy link
Contributor

commented Dec 8, 2018

Fix for #5804
tested manually by launching Cifar example + dependency analysis:

mvn dependency:tree | grep typetools

gives [INFO] | | +- net.jodah:typetools:jar:0.5.0:compile

@AlexDBlack

This comment has been minimized.

Copy link
Member

commented Dec 10, 2018

Thanks for the PR. What sort of testing have you done here?

@saudet saudet requested a review from AlexDBlack Dec 10, 2018

@smagellan

This comment has been minimized.

Copy link
Contributor Author

commented Dec 12, 2018

Manual tests only: checked if Cifar example launches under latest JDKs.
No luck with unit tests:
due this: #6742
and due memory leaks:

[----------] Global test environment tear-down
[==========] 2369 tests from 105 test cases ran. (5723 ms total)
[ PASSED ] 2369 tests.

=================================================================
==26702==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 72 byte(s) in 1 object(s) allocated from:
#0 0x7f3465a03ed0 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xe8ed0)
#1 0x55c370fcb1e7 in nd4j::NDArray::operator new(unsigned long) (/home/vladimir/IdeaProjects/deeplearning4j-fork/libnd4j/blasbuild/cpu/tests_cpu/layers_tests/runtests+0x1b1a1e7)

Direct leak of 72 byte(s) in 1 object(s) allocated from:
#0 0x7f3465a03ed0 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xe8ed0)
#1 0x55c370fcb1e7 in nd4j::NDArray::operator new(unsigned long) (/home/vladimir/IdeaProjects/deeplearning4j-fork/libnd4j/blasbuild/cpu/tests_cpu/layers_tests/runtests+0x1b1a1e7)
#2 0x55c376e82699 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::)(), char const) (/home/vladimir/IdeaProjects/deeplearning4j-fork/libnd4j/blasbuild/cpu/tests_cpu/layers_tests/runtests+0x79d1699)

@smagellan

This comment has been minimized.

Copy link
Contributor Author

commented Dec 18, 2018

@AlexDBlack, any opinions here?

@smagellan smagellan force-pushed the smagellan:update-typetools-to-allow-java9_10_11 branch from 8e8d54d to fd8cf63 Dec 18, 2018

@AlexDBlack

This comment has been minimized.

Copy link
Member

commented Dec 18, 2018

It looks good at first glance, but I need to build and test this manually, especially for anything scala related - UI and Spark.
I'll try to get to that today or tomorrow.

@smagellan smagellan force-pushed the smagellan:update-typetools-to-allow-java9_10_11 branch from cc98d06 to fd8cf63 Dec 19, 2018

@AlexDBlack
Copy link
Member

left a comment

Finally got to building and testing this manually (and checking dependency tree).
I didn't check Java 9+, but it certainly doesn't seem to break anything on Java 8.
Thanks for the PR. 👍

@AlexDBlack AlexDBlack merged commit 9e57a35 into deeplearning4j:master Dec 19, 2018

1 check was pending

codeclimate Code Climate is analyzing this code.
Details
@smagellan

This comment has been minimized.

Copy link
Contributor Author

commented Dec 20, 2018

Thank you for the great library!

printomi added a commit to printomi/deeplearning4j that referenced this pull request Jan 7, 2019

Updated typetools to allow java 9,10,11 (deeplearning4j#6819)
* updated net.jodah:typetools to 0.5.0

* updated net.jodah:typetools to 0.5.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.