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

Provide linear algebra utility #179

Merged
merged 22 commits into from
Nov 9, 2023

Conversation

joscao
Copy link
Contributor

@joscao joscao commented Oct 23, 2023

Provides basic linear algebra functionality necessary for array data dependency detection.

  • generate_row_echelon_form:

Calculates a row echelon form of a matrix, while allowing a conditional check before each division operation, which can also be specialized, as in our case integers are of interest with proper integer calculation.

  • Is modified version of a stack overflow post, is that a problem?

  • back_substitution

Applies a back substitution, provided by an upper triangular matrix onto a right hand side, again optional specialized division operation.

  • is_independent_system

Checks if a system of equations is independent, i.e. only one coefficient per row for the matrix or none.

  • yield_one_d_systems

Yields the one dimensional systems provided by an independent system of equations, disregards completely zero rows (matrix and rhs). Additionally catches (since it is generalized for inequalities (to be precise <=, >=) if the lhs row is all zeros and the rhs is non zero, will yield those as well.

- [x] bounds_of_one_d_system:

Calculates all unique bounds (C x > 0) and (Cx < 0) for a one dimensional system.
- [x] deal properly with zero in coefficient matrix C --> moving bounds to be (C x >= 0 and C x <= 0)

@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

Merging #179 (1754f10) into main (af8ec9f) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #179      +/-   ##
==========================================
+ Coverage   92.14%   92.16%   +0.01%     
==========================================
  Files          90       91       +1     
  Lines       16690    16730      +40     
==========================================
+ Hits        15379    15419      +40     
  Misses       1311     1311              
Flag Coverage Δ
lint_rules 96.22% <ø> (ø)
loki 92.13% <100.00%> (+0.02%) ⬆️
transformations 91.40% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
loki/analyse/util_linear_algebra.py 100.00% <100.00%> (ø)

- from interface was not clear how bounds function would deal with zero
  entry in left hand side, e.g. if 0 <= -1 would cause error
@joscao
Copy link
Contributor Author

joscao commented Oct 24, 2023

Remove unique bounds (C x > 0) and (Cx < 0) for a one dimensional system as it is unclear on how it would deal with zero left hand side , i.e. 0 <= 1 or 0 <= -1 , are those upper bounds, or would they result in failure. Therefore function design is lacking, decided to remove it completly!
f982037

@joscao joscao changed the title Draft: Provide linear algebra utility Provide linear algebra utility Oct 24, 2023
@joscao joscao mentioned this pull request Oct 24, 2023
2 tasks
Copy link
Collaborator

@reuterbal reuterbal left a comment

Choose a reason for hiding this comment

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

This looks great and very neatly done! I have not verified that the expected results of the tests are correct but trust your word on that ;-)

A remark on style and docstrings but none of them are showstoppers for this to be merged.

loki/analyse/util_linear_algebra.py Outdated Show resolved Hide resolved
loki/analyse/util_linear_algebra.py Outdated Show resolved Hide resolved
loki/analyse/util_linear_algebra.py Outdated Show resolved Hide resolved
@joscao
Copy link
Contributor Author

joscao commented Nov 2, 2023

Sorry, discovered a small bug upstream therefore needed to modify this utility. Now fixed. 1754f10

@reuterbal reuterbal added the ready for merge This PR has been approved and is ready to be merged label Nov 9, 2023
@reuterbal reuterbal merged commit ffe94af into ecmwf-ifs:main Nov 9, 2023
8 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for merge This PR has been approved and is ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants