Skip to content

Partitions with varying durations#97

Merged
JulStraus merged 5 commits into
mainfrom
js/partition_varying_duration
Jun 22, 2026
Merged

Partitions with varying durations#97
JulStraus merged 5 commits into
mainfrom
js/partition_varying_duration

Conversation

@JulStraus

Copy link
Copy Markdown
Collaborator

#95 introduced partitions based on durations for time periods. While the approach is satisfactory, it may be beneficial to provide varying partition durations when, e.g., the time structure is not regular. An example is the following time structure

ts = TwoLevel(2, 2, RepresentativePeriods(8760, [SimpleTimes(168, 1), SimpleTimes(24,1)]), op_per_strat=8760.)

where we utilize a peak day an a regular week.

This PR allows now the introduction of time profiles for specifying varying durations. This allows variations on the strategic, representative, operational scenario, and partition level.

@JulStraus JulStraus added the enhancement New feature or request label Jun 19, 2026
@JulStraus JulStraus requested a review from trulsf June 19, 2026 06:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR extends partition_duration to support varying partition durations by accepting time profiles (e.g., PartitionProfile, StrategicProfile) rather than only a fixed scalar duration, enabling irregular time structures (e.g., peak day + regular week) to define non-uniform partitioning.

Changes:

  • Generalized PartitionDurationIterator to store a TimeProfile of durations and query per-partition duration via profile indexing.
  • Moved PeriodPartition/partition-indexing traits into structures.jl and adjusted module include order to ensure profile types are available to utilities.
  • Updated/expanded partition-related tests, including new coverage for vector- and TimeProfile-based partition durations.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
test/runtests.jl Refactors partition tests into nested @testsets and adds vector/profile-based duration cases.
src/utils.jl Updates PartitionDurationIterator to accept TimeProfile durations and uses profile indexing during iteration.
src/TimeStruct.jl Reorders includes so profiles.jl loads before utils.jl.
src/structures.jl Relocates PeriodPartition and partition-indexing traits to be available before profiles/utilities.
src/partitions/strat_periods.jl Updates eltype dispatch for the new PartitionDurationIterator type parameters.
src/partitions/rep_periods.jl Updates eltype dispatch for the new PartitionDurationIterator type parameters.
src/partitions/opscenarios.jl Updates eltype dispatch for the new PartitionDurationIterator type parameters.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/utils.jl
Comment thread src/structures.jl
Comment thread test/runtests.jl
Comment thread test/runtests.jl
Comment thread test/runtests.jl
Comment thread test/runtests.jl
Comment thread test/runtests.jl
Comment thread test/runtests.jl
Comment thread test/runtests.jl
Comment thread test/runtests.jl

@trulsf trulsf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall the design looks good to me. I am a bit unsure about the use of NTuple{N,T} in the partitions as it may cause some type instability in the iterator (varying N). A Vector{T} may be an alternative to have type stability, but on the other hand cause more memory allocation problems. Maybe not a problem, but maybe worth thinking about.

@JulStraus

Copy link
Copy Markdown
Collaborator Author

Overall the design looks good to me. I am a bit unsure about the use of NTuple{N,T} in the partitions as it may cause some type instability in the iterator (varying N). A Vector{T} may be an alternative to have type stability, but on the other hand cause more memory allocation problems. Maybe not a problem, but maybe worth thinking about.

The NTuple{N,T} is required as equality does not really work well when you include a Vector. I realized that when working StrategicScenario where we did not know the value a priori. The problem is that a Vector is mutable while a NTuple is not.

I will move to Int in the test set to fix some problems with robustness in the test set, but I do not intend to include all of the other comments from Copilot as some of them do not make sense in my opinion.

@JulStraus JulStraus merged commit af78298 into main Jun 22, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants