Implement graph structure #254
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/run-benchmark |
|
abelsiqueira
force-pushed
the
graph-structure
branch
2 times, most recently
from
9b118a8 to
3352749
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #254 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 5 7 +2
Lines 236 201 -35
=========================================
- Hits 236 201 -35 ☔ View full report in Codecov by Sentry. |
abelsiqueira
force-pushed
the
graph-structure
branch
from
3352749 to
3161bcd
Compare
|
/run-benchmark |
|
datejada
reviewed
Nov 16, 2023
There was a problem hiding this comment.
One general question is if you are going to include changes for the representatives in this PR or if you are going to create a new one. Maybe we don't need changes in the rp i/o for the moment. I just wanted to ask since you mentioned it yesterday during the meeting.
src/io.jl
Outdated
| @@ -53,111 +79,33 @@ function create_parameters_and_sets_from_file(input_folder::AbstractString) | |||
|
|
|||
| # Parameter for profile of assets | |||
| assets_profile = Dict( | |||
| (assets[row.id], row.rep_period_id, row.time_step) => row.value for | |||
| (asset_data[row.id][1], row.rep_period_id, row.time_step) => row.value for | |||
There was a problem hiding this comment.
Is it possible to write explicitly that we are accessing the asset's name?
I mean, instead of asset_data[row.id][1] something like asset_data[row.id][name]
That will help with the readability of the code.
There was a problem hiding this comment.
Yes, this is easy but I missed it. Using label_for(graph, row.id) will work.
abelsiqueira
force-pushed
the
graph-structure
branch
from
3161bcd to
3c716ab
Compare
4 tasks
abelsiqueira
force-pushed
the
graph-structure
branch
from
3c716ab to
1e3e405
Compare
datejada
approved these changes
Nov 16, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request details
Describe the changes made in this pull request
Creates structures for assets and flows in the graph.
Creates a MetaGraph using those structures.
Stores the basic data from assets and flows in this graph.
The margin was changed from the default (92) to 100 to allow the ternary operators (
x ? y : z). Otherwise the formatter will split into an if-else and make the coverage decrease.List of related issues or pull requests
Closes #106, closes #105, closes #118
Collaboration confirmation
As a contributor I confirm