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

Add constructvariablecontainer! function #1039

Closed
wants to merge 1 commit into from

Conversation

blegat
Copy link
Member

@blegat blegat commented May 31, 2017

Depends on #1038
This could help creating different containers like AxisArrays #980

@chriscoey chriscoey added the Category: Containers Related to the Containers submodule label Jun 9, 2017
@chriscoey
Copy link
Contributor

we are doing some housekeeping, so bumping this. anything we need to discuss?

@blegat blegat force-pushed the constructvariablecontainer! branch from 59a90c9 to af3753b Compare June 10, 2017 08:43
@codecov
Copy link

codecov bot commented Jun 10, 2017

Codecov Report

Merging #1039 into master will increase coverage by 0.03%.
The diff coverage is 97.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1039      +/-   ##
==========================================
+ Coverage   90.63%   90.66%   +0.03%     
==========================================
  Files          18       18              
  Lines        4623     4617       -6     
==========================================
- Hits         4190     4186       -4     
+ Misses        433      431       -2
Impacted Files Coverage Δ
src/JuMP.jl 91.74% <ø> (-0.03%) ⬇️
src/macros.jl 91.79% <97.5%> (-0.05%) ⬇️
src/solvers.jl 95.84% <0%> (-0.01%) ⬇️
src/operators.jl 92.23% <0%> (+0.59%) ⬆️

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 02b8865...af3753b. Read the comment docs.

@codecov
Copy link

codecov bot commented Jun 10, 2017

Codecov Report

Merging #1039 into master will increase coverage by 0.03%.
The diff coverage is 97.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1039      +/-   ##
==========================================
+ Coverage   90.63%   90.66%   +0.03%     
==========================================
  Files          18       18              
  Lines        4623     4617       -6     
==========================================
- Hits         4190     4186       -4     
+ Misses        433      431       -2
Impacted Files Coverage Δ
src/JuMP.jl 91.74% <ø> (-0.03%) ⬇️
src/macros.jl 91.79% <97.5%> (-0.05%) ⬇️
src/solvers.jl 95.84% <0%> (-0.01%) ⬇️
src/operators.jl 92.23% <0%> (+0.59%) ⬆️

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 02b8865...af3753b. Read the comment docs.

@blegat
Copy link
Member Author

blegat commented Jun 10, 2017

@chriscoey Yes, I have just rebased it now that #1038 has been merged and I'd like to know what you think of it :)

@mlubin
Copy link
Member

mlubin commented Jun 10, 2017

There's no documentation, what does this change?

@blegat
Copy link
Member Author

blegat commented Jun 10, 2017

Yes documentation should be added. It basically calls constructvariablecontainer with the extra arguments. This allows to do the special treatment of SDP and Symmetric not so special. The issue is that this not done in evaluation of the code so we still have :SDP and :Symmetric as symbols. Therefore we should evaluate the part of extra that is used for building the container but not evalueate the part that is used to build each part yet... This still needs some thought but comments are welcome :)

@mlubin
Copy link
Member

mlubin commented Nov 25, 2017

Looks like this is stale, can we close?

@blegat
Copy link
Member Author

blegat commented Nov 26, 2017

Yes, we can try to make the change later

@blegat blegat closed this Nov 26, 2017
@odow odow deleted the constructvariablecontainer! branch December 30, 2018 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Containers Related to the Containers submodule
Development

Successfully merging this pull request may close these issues.

3 participants