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

Fix temporal resolution connection_ratio_out_in #1147

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

gnawin
Copy link
Collaborator

@gnawin gnawin commented Dec 2, 2024

Fixes #1146

Checklist before merging

  • Documentation is up-to-date
  • Unit tests have been added/updated accordingly
  • Code has been formatted according to SpineOpt's style
  • Unit tests pass

@gnawin
Copy link
Collaborator Author

gnawin commented Dec 2, 2024

Hi @DillonJ @manuelma @Tasqu ,

t indice:
I changed t resolution from lowest to highest, and also modified the constraint accordingly. Does it make sense to you?

s indice:
To me, it works as expected, if s is only parent. But when there is parent and child, it does not. Could you give some instructions on where/how to make changes?

Thanks!

@Tasqu Tasqu self-requested a review December 3, 2024 05:59
@gnawin
Copy link
Collaborator Author

gnawin commented Dec 5, 2024

I made some updates, could you have a further review @Tasqu? Thanks!

Another issue that I have not touched upon is the delays. Since delay basically introduces a separate resolution, we (@datejada and I) found out:

  • if the delay is multiples of the resolutions from the node, it works.
  • otherwise, it does not. What would be desired is that the constraint indices are the highest resolution between the nodes and the delay. If we decide to go that way, any idea of how the implementation works?

@Tasqu
Copy link
Member

Tasqu commented Dec 9, 2024

Another issue that I have not touched upon is the delays. Since delay basically introduces a separate resolution, we found out:

  • if the delay is multiples of the resolutions from the node, it works.
  • otherwise, it does not. What would be desired is that the constraint indices are the highest resolution between the nodes and the delay. If we decide to go that way, any idea of how the implementation works?

This might be tricky. I took a quick look, and I'm not sure why it wouldn't be working even if the delay isn't an exact match. My best guess is that it might have something to do with the to_time_slice and _to_time_slice functions, as those are used to figure out which time slice the delay should be pointing at.

Unfortunately, I most likely don't have time to look into this any further for now either :(

@gnawin gnawin force-pushed the 1146-fix-flex-reso-fix-ratio-out-in-connect branch from e1c7e04 to 9293cd4 Compare January 24, 2025 14:15
@gnawin
Copy link
Collaborator Author

gnawin commented Jan 24, 2025

We found out that the currently implementation does not scale well with a large model. It was discovered because this is causing a problem in the test for economic representation. This issue is not related to the economic representation itself, but because this case has the largest model in the dataset (hourly for a year). Here are the observations.

  1. By setting the model to hourly for a day, the resource usage is:
    [constraint_fix_ratio_out_in_connection_flow] 4.107311 seconds (6.92 M allocations: 359.169 MiB, 1.23% gc time, 98.64% compilation time)

    which is among the largest resource eaters, by comparable to e.g.,
    [constraint_fix_ratio_out_in_unit_flow] 4.330690 seconds (7.32 M allocations: 363.061 MiB, 1.10% gc time, 99.43% compilation
    time)

    far more than most , e.g.,
    [constraint_fix_ratio_out_out_unit_flow] 0.005151 seconds (730 allocations: 41.859 KiB, 97.33% compilation time)

  2. Notice that after the 1st run (i.e., after the complication), the usage drops to:
    [constraint_fix_ratio_out_in_connection_flow] 0.062868 seconds (659.58 k allocations: 50.421 MiB, 24.07% compilation time)

  3. After the complication, by setting the model to hourly for a month, the resource usage is:
    [constraint_fix_ratio_out_in_connection_flow] 39.678608 seconds (597.19 M allocations: 44.108 GiB, 13.65% gc time, 0.05% compilation time)
    [constraint_fix_ratio_out_in_unit_flow] 0.334825 seconds (3.56 M allocations: 286.618 MiB, 5.16% gc time, 3.65% compilation time)
    [constraint_fix_ratio_out_out_unit_flow] 0.000068 seconds (185 allocations: 15.609 KiB)

In summary, the implementation in this PR has a scalability problem, using both lots of time and memory. Maybe it is caused by type instability or other reasons, will investigate further.

@@ -111,17 +110,16 @@ end

function constraint_ratio_out_in_connection_flow_indices(m::Model, ratio_out_in)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @Tasqu @DillonJ @manuelma

I want to get the highest resolution between t_out and t_in (with delays), with their associated paths.
But this code does not return the constraints I want, do you see something weird in this indices function?

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.

Fix flexible time resolution in constraint fix_ratio_out_in_connection_flow
2 participants