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

Fixes for copy(::Model) #491

Merged
merged 1 commit into from
Jul 27, 2015
Merged

Fixes for copy(::Model) #491

merged 1 commit into from
Jul 27, 2015

Conversation

joehuchette
Copy link
Contributor

ref #490

@@ -214,6 +230,19 @@ function Base.copy(source::Model)
return dest
end

# Copy methods that I'm not sure where else to put
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd just put them in with the Variable section

@IainNZ
Copy link
Collaborator

IainNZ commented Jul 26, 2015

Related style of code I have in JuMPeR:
https://github.com/IainNZ/JuMPeR.jl/blob/aa24323313ba0db9d238419fe9fe4f318c9c91bd/src/solve.jl#L11-L24

Kinda funny that we had the variable copy-to-new-model code in JuMP but not this PR.

@joehuchette
Copy link
Contributor Author

I'm kind of tempted to turn of OS X travis until we get Homebrew figured out...

dest.colLower = source.colLower[:]
dest.colUpper = source.colUpper[:]
dest.colCat = source.colCat[:]

# callbacks and hooks
dest.callbacks = copy(source.callbacks)
Copy link
Member

Choose a reason for hiding this comment

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

Is it safe to copy callbacks? They refer to variables from the old model.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since callbacks are wrapped in types, not plain functions, I think this will result in a no method error, except if a user explicitly defines a copy method for their custom callback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Err, guess not. I'll just make this an error if callbacks are present.

joehuchette added a commit that referenced this pull request Jul 27, 2015
@joehuchette joehuchette merged commit 6a258e6 into master Jul 27, 2015
@IainNZ IainNZ deleted the model-copy branch September 5, 2015 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants