-
Notifications
You must be signed in to change notification settings - Fork 55
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
Fixed bug for composite DV #129
Conversation
Codecov Report
@@ Coverage Diff @@
## main #129 +/- ##
=======================================
Coverage 62.55% 62.56%
=======================================
Files 43 43
Lines 11182 11186 +4
=======================================
+ Hits 6995 6998 +3
- Misses 4187 4188 +1
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the bug fix looks good. I like the this was resolved offlinegetValues
change but I am not sure if it would break anybody's workflow? Maybe some people are getting the values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR's so good I had to approve it twice
Purpose
Apparently, I had hard-coded the initial DVs to zero. I didn't care at the time because in all my tests, the initial DVs happen to be zero anyways. But now I fixed it so it computes the correct initial DVs.
Also, I changed
getValues()
to return real values instead of complex. It didn't make sense to me to return any complex DVs, and I'm pretty sure the ESP/VSP versions will always return real values. The tests should all pass, and I'm somewhat confident in this change becausegetValues()
is not really used internally in DVGeo. But maintainers please double check that this works.Expected time until merged
ASAP
Type of change
Testing
N/A
Checklist
flake8
andblack
to make sure the code adheres to PEP-8 and is consistently formatted