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

Added elif statement MarginalizingNmat.solve to allow for 2D left_array #402

Merged
merged 3 commits into from
Dec 9, 2024

Conversation

JacobCT
Copy link

@JacobCT JacobCT commented Sep 26, 2024

Added elif statement made by @kgrunthal that is needed for Fe calculation and OS statistics. @bvgoncharov and I changed her original elif-condition from: elif right.ndim == 2 and left_array is not None and left_array.ndim == 2:, to: elif left_array is not None and right.ndim == left_array.ndim and right.ndim<=2:. This was done to include the case where both right and left_array have dimension 1, but they are not the same.

Matrix algebra should be double-checked to ensure that it is still applicable to this condition. It is possible that it is only valid with @kgrunthal's original conditions.

This PR fixes issue #401

…alculation and OS statistics. We changed her original elif-condition from: elif right.ndim == 2 and left_array is not None and left_array.ndim == 2:, to: elif left_array is not None and right.ndim == left_array.ndim and right.ndim<=2:. This was done in order to include the case where both right and left_array have dimension 1, but they are not the same.
@vhaasteren vhaasteren self-requested a review September 27, 2024 07:50
@vhaasteren vhaasteren changed the title Added elif statement to gp_signals.py Added elif statement MarginalizingNmat.solve to allow for 2D left_array Sep 27, 2024
vhaasteren
vhaasteren previously approved these changes Sep 27, 2024
@vhaasteren vhaasteren self-requested a review September 27, 2024 08:02
@vhaasteren vhaasteren dismissed their stale review September 27, 2024 08:02

Need a unit test

@vhaasteren
Copy link
Member

This change is ok. However, we need to add a unit test for the newly added code.

@JacobCT JacobCT marked this pull request as draft September 27, 2024 09:02
@JacobCT JacobCT changed the title Added elif statement MarginalizingNmat.solve to allow for 2D left_array DRAFT: Added elif statement MarginalizingNmat.solve to allow for 2D left_array Sep 27, 2024
@vhaasteren vhaasteren self-assigned this Nov 27, 2024
@vhaasteren vhaasteren changed the title DRAFT: Added elif statement MarginalizingNmat.solve to allow for 2D left_array Added elif statement MarginalizingNmat.solve to allow for 2D left_array Nov 27, 2024
@vhaasteren vhaasteren marked this pull request as ready for review November 27, 2024 19:25
Copy link

codecov bot commented Nov 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.36%. Comparing base (4f6154a) to head (ae7521e).
Report is 8 commits behind head on dev.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #402      +/-   ##
==========================================
+ Coverage   85.33%   85.36%   +0.02%     
==========================================
  Files          13       13              
  Lines        3158     3163       +5     
==========================================
+ Hits         2695     2700       +5     
  Misses        463      463              
Files with missing lines Coverage Δ
enterprise/signals/gp_signals.py 90.76% <100.00%> (+0.10%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 508e0d9...ae7521e. Read the comment docs.

@vhaasteren vhaasteren merged commit 4e84f13 into nanograv:dev Dec 9, 2024
15 checks passed
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.

2 participants