-
Notifications
You must be signed in to change notification settings - Fork 21
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 explicit asset timeframe partitions #1012
Conversation
Benchmark ResultsBenchmark in progress... |
@abelsiqueira so, from the benchmark we can see the performance problem when we have several representatives and we use the constraints over clustered year (inter-temporal constraints) by defining the timeframe partitions. The PR #1013 has the equivalent EU case study files in version 0.10.4 to have a reference. Also, Joaquim and Oscar recommended PProf.jl find type instabilities and bottlenecks in the code. I will give it a try on a different PR. I will keep you posted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, small simple change, pre-approved.
It is a bit concerning that this doesn't break the tests at all. It's either not affecting anything, or affecting things that don't change anything. Either way, it's another thing we have to create tests for.
Maybe we modify Norse
to remove one of the rows in assets-timeframe-partitions.csv
?
Co-authored-by: Abel Soares Siqueira <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1012 +/- ##
=======================================
Coverage 95.22% 95.22%
=======================================
Files 29 29
Lines 1151 1152 +1
=======================================
+ Hits 1096 1097 +1
Misses 55 55 ☔ View full report in Codecov by Sentry. |
Thanks @abelsiqueira! I have updated the Storage case since it used the Once the tests pass, I will merge them; thanks! |
This PR unifies the default value of the partitions for the representatives and the timeframe. So, if no data is defined in the partition files, the default partition is
uniform
of value1
.Related issues
Closes #1009
Checklist
I am following the contributing guidelines
Tests are passing
Lint workflow is passing
Docs were updated and workflow is passing