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

feat(bc): avoid unnecessary creation of RHS matrices for binding constraints #2077

Merged
merged 13 commits into from
Jul 25, 2024

Conversation

mabw-rte
Copy link
Contributor

Context:
We create many matrices, meanwhile some them are useless

Issue:
Over consumption of disk space

Solution:
New paradigm to avoid creating useless matrices

@mabw-rte mabw-rte self-assigned this Jun 25, 2024
@laurent-laporte-pro laurent-laporte-pro changed the title feat(BCs-matrices): avoid matrix duplication for binding constraints feat(bc-matrices): avoid matrix duplication for binding constraints Jun 25, 2024
@laurent-laporte-pro laurent-laporte-pro changed the title feat(bc-matrices): avoid matrix duplication for binding constraints feat(bc-matrices): avoid unnecessary creation of RHS matrices for binding constraints Jun 25, 2024
@laurent-laporte-pro laurent-laporte-pro changed the title feat(bc-matrices): avoid unnecessary creation of RHS matrices for binding constraints feat(bc): avoid unnecessary creation of RHS matrices for binding constraints Jun 25, 2024
@laurent-laporte-pro laurent-laporte-pro added enhancement New feature or request api REST API back-end labels Jun 27, 2024
@mabw-rte mabw-rte force-pushed the feature/1790-bc-bounds-matrices branch from 88c1ac2 to 7a2a55b Compare June 27, 2024 09:08
@pull-request-size pull-request-size bot added size/L and removed size/M labels Jun 27, 2024
@mabw-rte mabw-rte force-pushed the feature/1790-bc-bounds-matrices branch 2 times, most recently from 3c8a2f0 to d122d65 Compare June 27, 2024 14:11
@laurent-laporte-pro laurent-laporte-pro added this to the v2.17.3 milestone Jul 1, 2024
@mabw-rte mabw-rte force-pushed the feature/1790-bc-bounds-matrices branch from f3b5f93 to c3db377 Compare July 8, 2024 16:05
@mabw-rte mabw-rte force-pushed the feature/1790-bc-bounds-matrices branch from c3db377 to 1286f5b Compare July 11, 2024 23:50
@mabw-rte mabw-rte requested a review from MartinBelthle July 12, 2024 00:07
@mabw-rte mabw-rte force-pushed the feature/1790-bc-bounds-matrices branch from 1286f5b to b9e9bd7 Compare July 12, 2024 08:14
Copy link
Contributor

@MartinBelthle MartinBelthle left a comment

Choose a reason for hiding this comment

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

Keep going you've already coded a big part of the feature

Copy link
Member

@sylvlecl sylvlecl left a comment

Choose a reason for hiding this comment

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

I think we need to change slightly the implementation:
the level of abstraction in the command is not appropriate, it should manipulate nodes and not paths. Manipulation of paths should be hidden inside the nodes as much as possible.

We need to add copy and move operations to the nodes in order to implement this.

@mabw-rte mabw-rte force-pushed the feature/1790-bc-bounds-matrices branch 2 times, most recently from 1a487dc to a01cedd Compare July 22, 2024 16:11
@mabw-rte mabw-rte requested a review from MartinBelthle July 23, 2024 11:25
Copy link
Contributor

@MartinBelthle MartinBelthle left a comment

Choose a reason for hiding this comment

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

Last round of comment for me

@mabw-rte mabw-rte force-pushed the feature/1790-bc-bounds-matrices branch from d6ae88a to e41786e Compare July 23, 2024 21:42
Copy link
Member

@sylvlecl sylvlecl left a comment

Choose a reason for hiding this comment

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

Implementation is better but I think we should add unit tests, not only integration tests!
In particular there might be an issue for the new copy and rename methdos

@mabw-rte mabw-rte force-pushed the feature/1790-bc-bounds-matrices branch from 2582a95 to 5cafd86 Compare July 24, 2024 23:56
sylvlecl
sylvlecl previously approved these changes Jul 25, 2024
@sylvlecl sylvlecl merged commit beaa4c1 into dev Jul 25, 2024
7 checks passed
@sylvlecl sylvlecl deleted the feature/1790-bc-bounds-matrices branch July 25, 2024 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants