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.

Sign up for GitHub

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

Jump to bottom

Finalize NRSE #495

Merged
merged 21 commits into from
Feb 15, 2024
Merged

Finalize NRSE #495

merged 21 commits into from
Feb 15, 2024

Conversation

mgovers
Copy link
Member

@mgovers mgovers commented Feb 15, 2024 •
edited by TonyXiang8787
Loading

  • prepare_matrix_and_rhs:
    • BUG: (L250) also clear fill-in
    • (L328) no prefactorize is needed.
  • The NRSEGainBlock (L64-L67):
    • qt should be (hermitian) transposed
    • qt should be directly obtained from hermitian transpose instead of per element
  • Run_state_estimation:
    • (L163) run solve directly using prefactorize_and_solve
  • fix dense grouped index vector not initialized by default
  • sensor-update case for NRSE
  • distribution-case for NRSE and iterative linear
  • single-line-load case

Relates to #369

@mgovers
fix grouped idx vector
1c4a9d0 
Signed-off-by: Martijn Govers <[email protected]>
@mgovers mgovers added the bug Something isn't working label Feb 15, 2024
@mgovers mgovers self-assigned this Feb 15, 2024
mgovers and others added 7 commits February 15, 2024 10:54
@mgovers
fix fill in
b2bb7e4 
Signed-off-by: Martijn Govers <[email protected]>
@mgovers
resolve comments
d95a993 
Signed-off-by: Martijn Govers <[email protected]>
@mgovers
use transpose
04ed0cb 
Signed-off-by: Martijn Govers <[email protected]>
@mgovers
always on export_output pytest
bbe9760 
Signed-off-by: Martijn Govers <[email protected]>
@TonyXiang8787
fix ci error
4ea1ef6 
Signed-off-by: Tony Xiang <[email protected]>
@TonyXiang8787
reformat validation case for readability and inspection
6f85a4f 
Signed-off-by: Tony Xiang <[email protected]>
@TonyXiang8787
add sensor update case for NRSE
812b439 
Signed-off-by: Tony Xiang <[email protected]>
@TonyXiang8787 TonyXiang8787 changed the title Fix NRSE after math review Finalize NRSE Feb 15, 2024
@TonyXiang8787
Copy link
Member

After examination I have concluded that the case sensor-update has indeed another local minimum for NRSE. Thus a new test case created.

@mgovers
Copy link
Member Author

mgovers commented Feb 15, 2024

After examination I have concluded that the case sensor-update has indeed another local minimum for NRSE. Thus a new test case created.

ok, clear. i'm working on the block matrix thingy. should i do that in this PR or a separate one?

@TonyXiang8787
Copy link
Member

After examination I have concluded that the case sensor-update has indeed another local minimum for NRSE. Thus a new test case created.

ok, clear. i'm working on the block matrix thingy. should i do that in this PR or a separate one?

Just push to here. We try to finish everything.

@mgovers
Copy link
Member Author

mgovers commented Feb 15, 2024

After examination I have concluded that the case sensor-update has indeed another local minimum for NRSE. Thus a new test case created.

ok, clear. i'm working on the block matrix thingy. should i do that in this PR or a separate one?

Just push to here. We try to finish everything.

i would like to do the version bump + remove experimental flag in a separate PR, just to really make sure that that version bump is extremely minimal and that we don't end up finding something again before pushing

mgovers and others added 12 commits February 15, 2024 15:02
@mgovers
better block matrix interface
ecba3d0 
Signed-off-by: Martijn Govers <[email protected]>
@mgovers
Merge branch 'bugfix/nrse-maths-review' of https://github.com/PowerGr…
e9ee711 
…idModel/power-grid-model into bugfix/nrse-maths-review
@mgovers
do not use (void)
4bfd4d4 
Signed-off-by: Martijn Govers <[email protected]>
@mgovers
rename test cases
32c2f6d 
Signed-off-by: Martijn Govers <[email protected]>
@mgovers
add NRSE back to single line + load
789b613 
Signed-off-by: Martijn Govers <[email protected]>
@TonyXiang8787
xfail for single line load
e9cd547 
Signed-off-by: Tony Xiang <[email protected]>
@TonyXiang8787
do not run test case twice
e916593 
Signed-off-by: Tony Xiang <[email protected]>
@TonyXiang8787
single line load case only for ILSE
9be7403 
Signed-off-by: Tony Xiang <[email protected]>
@TonyXiang8787
[skip ci] distribution case not working yet
c0dc1b3 
Signed-off-by: Tony Xiang <[email protected]>
@TonyXiang8787
[skip ci] rename
dcaf0c9 
Signed-off-by: Tony Xiang <[email protected]>
@TonyXiang8787
[skip ci] rename
aeabc40 
Signed-off-by: Tony Xiang <[email protected]>
@TonyXiang8787
remove single line load again
4763d8d 
Signed-off-by: Tony Xiang <[email protected]>
@TonyXiang8787 TonyXiang8787 marked this pull request as ready for review February 15, 2024 15:03
TonyXiang8787
TonyXiang8787 previously approved these changes Feb 15, 2024
View reviewed changes
@mgovers
Copy link
Member Author

mgovers commented Feb 15, 2024

I see no more weird/unexpected things. CI appears to go OK both locally and remotely. Hereby enabling automerge

@mgovers mgovers enabled auto-merge February 15, 2024 15:20
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@mgovers mgovers merged commit 9bb0ed4 into main Feb 15, 2024
27 checks passed
@mgovers mgovers deleted the bugfix/nrse-maths-review branch February 15, 2024 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TonyXiang8787 TonyXiang8787 TonyXiang8787 approved these changes

Assignees

@mgovers mgovers

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mgovers @TonyXiang8787