Skip to content

Conversation

@trulsf
Copy link
Member

@trulsf trulsf commented Nov 17, 2025

In some algorithmic work where we use iterators inside algorithmic loops the calculation of total_duration dominates time usage and it may be worthwhile to cache these as part of the structure.

@trulsf trulsf requested review from JulStraus and hellemo November 17, 2025 08:37
Copy link
Collaborator

@JulStraus JulStraus left a comment

Choose a reason for hiding this comment

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

Not too many comments from my end. We had it included for RepresentativePeriods beforehand (although with a different meaning). The usage of an internal constructor is also beneficial.

One thing we should however be clearer on is the differentiation on what the function does for a TwoLevel and the different "operational" time structures. This boils essentially down to the differentiation between the duration of a strategic period and the duration of an operational period. I could come one within November with a suggestion for an improved documentation for it.

Edit: You forgot the JuliaFormatter ;) Should CalendarTimes also get the update?

Copy link
Collaborator

@JulStraus JulStraus left a comment

Choose a reason for hiding this comment

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

LGTM.

@trulsf trulsf merged commit f433cce into main Nov 21, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants