Add optimization test with FD derivatives#487
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #487 +/- ##
===========================================
- Coverage 82.99% 54.22% -28.78%
===========================================
Files 1 1
Lines 147 225 +78
===========================================
Hits 122 122
- Misses 25 103 +78 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Which one should be merged first between this PR and #486 ? Both add the same two new files, does it make sense attach them to one PR over the other? |
|
This is on top of #486, let's wait for that one first. |
| # Complex step divides by the imaginary part of the step, so a purely | ||
| # real step would silently yield NaN gradients. | ||
| if self.sensType == "cs" and np.imag(self.sensStep) == 0: | ||
| raise ValueError(f"The complex step size must have a nonzero imaginary part, got {self.sensStep}.") |
There was a problem hiding this comment.
The docstring says that sensStep is the step size, and should be a float. So we need to either
- Update the docstring
- Change the implementation so that we take in a float and do
self.sensStep = 1j * sensStepwhen using complex-step
Option 2 would be my slight preference.
There was a problem hiding this comment.
Also, if we keep the implementation as is and update the docstring, shouldn't we also check that sensStep is purely imaginary (i.e np.real(self.sensStep) == 0?)
Purpose
Addresses part of #256.
Expected time until merged
A few days.
Type of change
Testing
Checklist
ruff checkandruff formatto make sure the Python code adheres to PEP-8 and is consistently formattedfprettifyor C/C++ code withclang-formatas applicable