Opened 6 years ago
Closed 5 years ago
#2948 closed enhancement (done)
Symbolic differentiation
Reported by: | gkronber | Owned by: | gkronber |
---|---|---|---|
Priority: | medium | Milestone: | HeuristicLab 3.3.16 |
Component: | Problems.DataAnalysis.Symbolic | Version: | trunk |
Keywords: | Cc: |
Description
Sometimes it can be helpful to look at derivatives of a model. It is rather straightforward to implement symbolic differentiation using our tree representation of expressions.
Change History (32)
comment:1 Changed 6 years ago by gkronber
- Owner set to gkronber
- Status changed from new to accepted
- Version set to trunk
comment:2 Changed 6 years ago by gkronber
comment:3 Changed 6 years ago by gkronber
r16207: added new unit test class to project (combine with previous commit)
comment:4 Changed 6 years ago by gkronber
r16213: changed unit test cases to assert results of derivative calculations. Fixed bug in deriving sqrt(x)
comment:5 Changed 6 years ago by gkronber
- Owner changed from gkronber to mkommend
- Status changed from accepted to reviewing
comment:6 Changed 6 years ago by gkronber
r16216: removed string interpolation
comment:7 follow-ups: ↓ 9 ↓ 11 Changed 6 years ago by mkommend
- Owner changed from mkommend to gkronber
- Status changed from reviewing to assigned
Review Comments
Derivative Calculator
- Method Derive (line 29) - Although ADFs are virtually never used at least a check that they are not present should be included before accessing tree.Root.GetSubtree(0).GetSubtree(0).
- Static symbol variables (line 38-42) - Should be static readonly to avoid reassignment.
- Should a shortcut for deriving f(x1, ... x10) / dy be included ( f'(x1,...,x10)/dy = 0) or is this automatically handled by simplification of a constant expression regardless of its length?
- Method Derive (line 74) - Is there a specific reason why unary multiplication is not handled?
- Method Derive (line 90) - Is there a specific reason why divisions with more than 2 arguments are not handled?
- Method Derive (line 136) - What about the tangent symbol that can be expressed as sin() / cos()?
- Method IsCompatible (line 172) - This method is never used!
DeriveTest
- Reviewed unit tests
- Some of them look overly complicated (due to our simplifier) and I had to use wolfram alpha for verification.
comment:8 Changed 6 years ago by gkronber
r16294: worked on review comments.
comment:9 in reply to: ↑ 7 Changed 6 years ago by gkronber
implemented the following review comments with r16294:
Replying to mkommend:
- Method Derive (line 29) - Although ADFs are virtually never used at least a check that they are not present should be included before accessing tree.Root.GetSubtree(0).GetSubtree(0).
- Static symbol variables (line 38-42) - Should be static readonly to avoid reassignment.
- Method Derive (line 74) - Is there a specific reason why unary multiplication is not handled?
- Method Derive (line 90) - Is there a specific reason why divisions with more than 2 arguments are not handled?
- Method Derive (line 136) - What about the tangent symbol that can be expressed as sin() / cos()?
Still open:
- Should a shortcut for deriving f(x1, ... x10) / dy be included ( f'(x1,...,x10)/dy = 0) or is this automatically handled by simplification of a constant expression regardless of its length?
- Method IsCompatible (line 172) - This method is never used!
It can be used by classes working with DerivativeCalculator. The pattern is the same for the AutoDiffConverter.
comment:10 Changed 6 years ago by gkronber
comment:11 in reply to: ↑ 7 Changed 6 years ago by gkronber
Replying to mkommend:
Review Comments
Derivative Calculator
- Should a shortcut for deriving f(x1, ... x10) / dy be included ( f'(x1,...,x10)/dy = 0) or is this automatically handled by simplification of a constant expression regardless of its length?
Checking whether the variable is used in ISymbolicExpressionTree would necessitate an iteration over all tree nodes. If Derive(ISymbolicExpressionTree tree, string variableName) is called for a variableName which does not occur in the tree the result is automatically a constant zero tree. I added a unit test with r16494 to assert this.
comment:12 Changed 6 years ago by gkronber
- Owner changed from gkronber to mkommend
- Status changed from assigned to reviewing
comment:13 Changed 6 years ago by gkronber
Not supported:
- Factor variables
- Abs
- AQ, Cube, CubeRoot
- time series symbols
- Boolean operators and conditionals
- Power, root (with rounding)
- all special functions
comment:14 Changed 6 years ago by gkronber
- Owner changed from mkommend to gkronber
- Status changed from reviewing to assigned
comment:15 Changed 6 years ago by gkronber
- Status changed from assigned to accepted
comment:16 Changed 6 years ago by gkronber
r16543: added support for deriving cube, cuberoot, abs, analytic-quotient
comment:17 Changed 6 years ago by gkronber
- Owner changed from gkronber to mkommend
- Status changed from accepted to reviewing
comment:18 Changed 6 years ago by gkronber
- Owner changed from mkommend to gkronber
- Status changed from reviewing to assigned
comment:19 Changed 6 years ago by gkronber
r16737: fixed a bug for products with >2 factors
comment:20 Changed 6 years ago by gkronber
- Owner changed from gkronber to mkommend
- Status changed from assigned to reviewing
comment:21 Changed 6 years ago by mkommend
- Owner changed from mkommend to gkronber
- Status changed from reviewing to readytorelease
Reviewed all changesets of this ticket and the code looks good. However, I haven't tested it.
comment:22 Changed 5 years ago by mkommend
- Keywords depends-2520 mergewith-(2915 2866 2958 2966) added
comment:23 Changed 5 years ago by mkommend
r17066: Merged 16206, 16207, 16213, 16216, 16294 to stable.
comment:24 Changed 5 years ago by mkommend
- Keywords merged added; depends-2520 mergewith-(2915 2866 2958 2966) removed
r17102: Merged 16358, 16494, 16543, 16737 into stable.
comment:25 Changed 5 years ago by gkronber
- Status changed from readytorelease to assigned
Hyperbolic tangent is not supported.
comment:26 Changed 5 years ago by gkronber
- Status changed from assigned to reviewing
r17125: added support for tanh and a unit test case
comment:27 Changed 5 years ago by gkronber
- Owner changed from gkronber to mkommend
Please review / release r17125
comment:28 Changed 5 years ago by mkommend
Reviewed r17125.
comment:29 Changed 5 years ago by mkommend
- Owner changed from mkommend to gkronber
- Status changed from reviewing to readytorelease
comment:30 Changed 5 years ago by abeham
- Keywords merged removed
comment:31 Changed 5 years ago by gkronber
- Keywords merged added
comment:32 Changed 5 years ago by jkarder
- Keywords merged removed
- Resolution set to done
- Status changed from readytorelease to closed
r16206: added first implementation of symbolic differentiation as well as a test case (to be checked manually)