Skip to content

Small changes for the release#291

Merged
nouiz merged 9 commits intoTheano:masterfrom
abergeron:for_release
Nov 24, 2016
Merged

Small changes for the release#291
nouiz merged 9 commits intoTheano:masterfrom
abergeron:for_release

Conversation

@abergeron
Copy link
Copy Markdown
Member

@abergeron abergeron commented Nov 22, 2016

No description provided.

nouiz
nouiz reviewed Nov 23, 2016
View reviewed changes
src/CMakeLists.txt
COMPILE_FLAGS "-DGPUARRAY_BUILDING_DLL -DGPUARRAY_SHARED"
INSTALL_NAME_DIR ${CMAKE_INSTALL_PREFIX}/lib
MACOSX_RPATH OFF
VERSION 0.0
Copy link
Copy Markdown
Member

@nouiz nouiz Nov 23, 2016

Choose a reason for hiding this comment

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

How we can get access to that version in Theano? Does this version if an interface change or any code version upgrade? I think we need to keep that information available in the version in some way.

Copy link
Copy Markdown
Member Author

@abergeron abergeron Nov 23, 2016

Choose a reason for hiding this comment

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

Normally this automatically handled through shared library versioning.

If you change an interface, you bump the first number, if you add new things you bump the second number.

@nouiz
Copy link
Copy Markdown
Member

nouiz commented Nov 23, 2016

but the PR on theano just remove all versioning stuff. It should use the
new versioning interface instead.

On Wed, Nov 23, 2016 at 11:45 AM, abergeron [email protected]
wrote:

@abergeron commented on this pull request.

In src/CMakeLists.txt #291:

@@ -87,6 +87,7 @@ set_target_properties(gpuarray PROPERTIES
COMPILE_FLAGS "-DGPUARRAY_BUILDING_DLL -DGPUARRAY_SHARED"
INSTALL_NAME_DIR ${CMAKE_INSTALL_PREFIX}/lib
MACOSX_RPATH OFF

  • VERSION 0.0

Normally this automatically handled through shared library versioning.

If you change an interface, you bump the first number, if you add new
things you bump the second number.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#291, or mute the thread
https://github.com/notifications/unsubscribe-auth/AALC-7s8_nQgmdru2a3FOBhUFAiDwilQks5rBG3HgaJpZM4K6CSV
.

@abergeron
Copy link
Copy Markdown
Member Author

abergeron commented Nov 23, 2016

The shared library linker of the OS will handle this if we put the versioning in the library like this.

This is why we don't need custom handling in Theano anymore.

@nouiz
Copy link
Copy Markdown
Member

nouiz commented Nov 24, 2016

Test for the set strides are missing.

This was referenced Nov 24, 2016
Closed
Closed
@abergeron
Copy link
Copy Markdown
Member Author

abergeron commented Nov 24, 2016

There is a test now (and the same checks that numpy does).

@nouiz
Copy link
Copy Markdown
Member

nouiz commented Nov 24, 2016

Except for the docstring, we can merge this PR.

@nouiz nouiz merged commit fc649da into Theano:master Nov 24, 2016
@abergeron abergeron deleted the for_release branch November 28, 2016 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nouiz nouiz nouiz left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@abergeron @nouiz