[WIP] Add symbolic integral #240

ashutosh-b-b · 2021-05-21T06:21:30Z

Need to change expand_derivatives in order to handle derivative of an integral.

ashutosh-b-b · 2021-05-21T06:22:34Z

@ChrisRackauckas @zoemcc

src/integral.jl

ChrisRackauckas · 2021-05-21T10:53:08Z

This definition only works for 1-dimensional integrals. I think it instead needs to be specified by boundary domains?

ashutosh-b-b · 2021-05-21T14:27:53Z

This definition only works for 1-dimensional integrals. I think it instead needs to be specified by boundary domains?

Right, so instead I ll go with the domains we have in MTK?

ChrisRackauckas · 2021-05-21T19:26:06Z

Yeah, the DomainSets.jl domains.

codecov-commenter · 2021-05-21T19:31:49Z

Codecov Report

Merging #240 (0dae216) into master (5d8dfeb) will decrease coverage by 0.51%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #240      +/-   ##
==========================================
- Coverage   70.78%   70.26%   -0.52%     
==========================================
  Files          12       13       +1     
  Lines        1085     1093       +8     
==========================================
  Hits          768      768              
- Misses        317      325       +8

Impacted Files	Coverage Δ
src/integral.jl	`0.00% <0.00%> (ø)`

Continue to review full report at Codecov.

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

YingboMa · 2021-05-21T22:08:54Z

src/integral.jl

+
+function Base.show(io::IO, I::Integral)
+    print(io, "Integral(", I.x, ")")
+    print(io,"upper_bound: ")


Use consistent indentation of a space after a comma and no space before the comma.

YingboMa · 2021-05-21T22:09:02Z

test/integral.jl

+
+@variables x y
+
+I1 = Integral(x , y , 0)


ChrisRackauckas · 2021-05-22T12:44:54Z

Comments @tinosulzer and @zoemcc?

ashutosh-b-b · 2021-05-23T11:03:25Z

@ChrisRackauckas @zoemcc expand_derivatives should be change to handle the derivative of an integral case right?

ChrisRackauckas · 2021-05-23T13:03:13Z

Should it? I am not sure it should. @YingboMa

ashutosh-b-b · 2021-05-23T14:04:49Z

It applies the chain rule over a function in Differential , so if we have D(cos(u(x))) where D is a Differential(x) what we get is -Differential(x)(u(x))*sin(u(x)), It applies the same when we have an integral inside a differential

YingboMa · 2021-05-23T16:03:39Z

Should it?

I think it should try to expand the derivative for an integral as well, since that's how derivative, jacobian, and etc are implemented. Is the concern that it could limit expressing integro-differential equations? I am not familiar with those, but it seems to me that we don't usually have a derivative in front of the integral in those equations, right? Even so, we don't really need expand_derivative when specifying the equations.

valentinsulzer · 2021-05-23T21:56:43Z

What's there looks sensible, can get updated as needed when specific use cases come up
Should we add a test along the lines of

@variables t, x
@variables u(..)

domain = ...
Ix = Integral(x, domain)
Ix(u(t,x))

@test ...

ChrisRackauckas · 2021-05-23T22:11:06Z

I think it should try to expand the derivative for an integral as well, since that's how derivative, jacobian, and etc are implemented. Is the concern that it could limit expressing integro-differential equations? I am not familiar with those, but it seems to me that we don't usually have a derivative in front of the integral in those equations, right? Even so, we don't really need expand_derivative when specifying the equations.

I see, I thought the question was if expand_derivative should do symbolic integration, which seemed weird. Okay yes, makes sense to limit swap.

shashi · 2021-05-31T15:55:14Z

I can infer from the code that we have a way to say "integral wrt x from a to b" but not "integral of something wrt x from a to b"... What gives? Is an instance of Integral supposed to become callable?

valentinsulzer · 2021-05-31T16:14:51Z

That seems like the right way to do it for consistency with Differential

ashutosh-b-b · 2021-06-10T21:26:50Z

I made some changes for Derivative of an Integral.

shashi · 2021-06-11T23:59:19Z

src/diff.jl

+                    t2 = D(a)
+                    c -= t1*t2
+                elseif isa(value(b) , Term)
+                    t1 = replaceSym(operation(arg).x ,value(b) , inner_function )


formatting needs to be cleaned up, we also never use camelCase function names. Good to stick to the package's style when you're contributing to it. Where is the expand_derivatives_ function...?

you could use substitute here.

Yes, I infact removed replaceSym

YingboMa · 2021-06-12T23:15:19Z

test/integral.jl

+D = Differential(x)
+Dxx = Differential(x)^2
+I = Integral(x, ClosedInterval(1 , v(x)))
+eq = D((I(u(x,t)) )) ~ 0


YingboMa · 2021-06-12T23:15:26Z

test/integral.jl

+eq = D((I(u(x,t)) )) ~ 0
+O = expand_derivatives(eq.lhs)
+println(O)
+eq = Dxx((I(u(x,t)) )) ~ 0


YingboMa · 2021-06-12T23:15:35Z

test/integral.jl

+println(O)
+eq = Dxx((I(u(x,t)) )) ~ 0
+O = expand_derivatives(eq.lhs)
+println(O)


YingboMa · 2021-06-12T23:16:52Z

src/integral.jl

+    print(io, "Integral(", I.x, ", ", I.domain, ")")
+end
+
+Base.:(==)(I1::Integral, I2::Integral) = (isequal(I1.x, I2.x) && isequal(I1.domain, I2.domain))


When comparing numbers, we should use == instead of isequal. Consider -0.0 vs 0.0.

Ah, I guess domains could be symbolic.

YingboMa · 2021-06-12T23:17:22Z

src/integral.jl

+end
+
+Base.:(==)(I1::Integral, I2::Integral) = (isequal(I1.x, I2.x) && isequal(I1.domain, I2.domain))
+(D::Differential)(I::Integral{X,T}) where{X,T} = I∘D


Indentation.

YingboMa · 2021-06-12T23:17:42Z

src/diff.jl

@@ -113,6 +113,26 @@ function expand_derivatives(O::Symbolic, simplify=false; occurances=nothing)
                    return expand_derivatives(operation(arg)(inner), simplify)
                end
            end
+        elseif isa(operation(arg) , Integral)


Whitespace.

YingboMa · 2021-06-12T23:18:03Z

src/diff.jl

+        elseif isa(operation(arg) , Integral)
+            if isa(operation(arg).domain , AbstractInterval)
+                domain = operation(arg).domain
+                a , b = DomainSets.endpoints(domain)


Whitespace.

YingboMa · 2021-06-12T23:18:15Z

src/diff.jl

+                    t2 = D(a)
+                    c -= t1*t2
+                end
+                if isa(value(b) , Term)


Whitespace.

YingboMa · 2021-06-12T23:19:13Z

src/diff.jl

+                a , b = DomainSets.endpoints(domain)
+                c = 0
+                inner_function = expand_derivatives(arguments(arg)[1])
+                if isa(value(a), Term)


Why isa Term? Shouldn't it be istree? Check SymbolicUtils' docs.

YingboMa · 2021-06-12T23:21:47Z

Remember to squash when merging this PR.

YingboMa · 2021-06-13T18:27:48Z

Need to resolve the merge conflict.

YingboMa · 2021-06-13T18:29:07Z

test/integral.jl

+I = Integral(y, ClosedInterval(1, v(x)))
+eq = D((I(u(x,y)))) ~ 0
+eq_test = I(D(u(x,y))) + D(v(x))*u(x, v(x))
+@test isequal(expand_derivatives(eq.lhs), Symbolics.value(eq_test)) == true


== true is redundant

YingboMa · 2021-06-14T00:07:12Z

test/integral.jl

+I = Integral(y, ClosedInterval(1, 5))
+eq = D((I(u(x,y)))) ~ 0
+eq_test = I(D(u(x,y)))
+@test isequal(expand_derivatives(eq.lhs), Symbolics.value(eq_test))


Just using Symbolics.derivative could be fine.

YingboMa · 2021-06-14T00:09:28Z

test/integral.jl

+eq_test = I(D(u(x,y))) + D(v(x))*u(x, v(x))
+@test isequal(expand_derivatives(eq.lhs), Symbolics.value(eq_test))
+
+I = Integral(y, ClosedInterval(1, v(x)))


What if you test an integral with the integrand u^2? Does it expand the derivative?

Yes, it does expand. Should I add this as a test?

I did, in the last commit.

YingboMa · 2021-06-15T16:18:48Z

CI failed.

YingboMa · 2021-06-15T19:26:05Z

src/diff.jl

@@ -171,7 +171,7 @@ function expand_derivatives(O::Symbolic, simplify=false; occurances=nothing)
            x = +((!_iszero(c) ? vcat(c, exprs) : exprs)...)
            return simplify ? SymbolicUtils.simplify(x) : x
        end
-    elseif isa(operation(O), Integral )
+    elseif istree(O) && isa(operation(O), Integral )


Could you be more careful about whitespace?

I m sorry, I fixed that.

ashutosh-b-b added 2 commits May 21, 2021 11:47

Basic integral with equality check

b3401bf

Export integral and add test

9ada0ec

ashutosh-b-b changed the title ~~[WIP] Addition for symbolic integral~~ [WIP] Add symbolic integral May 21, 2021

ChrisRackauckas reviewed May 21, 2021

View reviewed changes

src/integral.jl Outdated Show resolved Hide resolved

Fix Typo

0ed131b

YingboMa reviewed May 21, 2021

View reviewed changes

test/integral.jl Outdated

@variables x y

I1 = Integral(x , y , 0)

Copy link

Member

YingboMa May 21, 2021

Choose a reason for hiding this comment

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

Same here

ashutosh-b-b added 2 commits May 22, 2021 11:07

Using Domains for integral

4ac71b3

Fix indentation and isequal

0f2fe53

Add derivative of integral

0dae216

ashutosh-b-b added 3 commits June 10, 2021 02:51

Add replaceSym function

76871b3

Update expand_derivatives for integral and Leibniz rule

59c7fae

Modify expand_derivatives

53b3227

Merge branch 'master' into SymbolicIntegral

4c7bd4e

shashi reviewed Jun 11, 2021

View reviewed changes

YingboMa reviewed Jun 12, 2021

View reviewed changes

test/integral.jl Outdated

eq = D((I(u(x,t)) )) ~ 0

O = expand_derivatives(eq.lhs)

println(O)

eq = Dxx((I(u(x,t)) )) ~ 0

Copy link

Member

YingboMa Jun 12, 2021

Choose a reason for hiding this comment

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

Here.

YingboMa reviewed Jun 12, 2021

View reviewed changes

test/integral.jl Outdated

println(O)

eq = Dxx((I(u(x,t)) )) ~ 0

O = expand_derivatives(eq.lhs)

println(O)

Copy link

Member

YingboMa Jun 12, 2021

Choose a reason for hiding this comment

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

Why print?

YingboMa reviewed Jun 12, 2021

View reviewed changes

src/diff.jl Outdated

t2 = D(a)

c -= t1*t2

end

if isa(value(b) , Term)

Copy link

Member

YingboMa Jun 12, 2021

Choose a reason for hiding this comment

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

Whitespace.

YingboMa reviewed Jun 12, 2021

View reviewed changes

ashutosh-b-b added 2 commits June 13, 2021 12:13

Add tests, use istree

061043c

Fix isequal , add tests

a508dbf

ashutosh-b-b requested review from shashi, YingboMa and ChrisRackauckas June 13, 2021 16:19

YingboMa reviewed Jun 13, 2021

View reviewed changes

ashutosh-b-b and others added 2 commits June 14, 2021 01:32

Merge branch 'master' into SymbolicIntegral

29ff905

remove ==true

7f4d43f

YingboMa reviewed Jun 14, 2021

View reviewed changes

ashutosh-b-b added 2 commits June 15, 2021 02:23

Add u^2 test

60c1323

fix test

153463a

ashutosh-b-b requested a review from YingboMa June 15, 2021 15:31

add istree

d96f248

YingboMa reviewed Jun 15, 2021

View reviewed changes

fix whitespace

320a71d

YingboMa merged commit ed5186b into JuliaSymbolics:master Jun 15, 2021

CarloLucibello mentioned this pull request Mar 14, 2022

Integral is not documented #547

Open

[WIP] Add symbolic integral #240

[WIP] Add symbolic integral #240

Conversation

ashutosh-b-b commented May 21, 2021

ashutosh-b-b commented May 21, 2021

ChrisRackauckas commented May 21, 2021

ashutosh-b-b commented May 21, 2021

ChrisRackauckas commented May 21, 2021

codecov-commenter commented May 21, 2021 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrisRackauckas commented May 22, 2021

ashutosh-b-b commented May 23, 2021

ChrisRackauckas commented May 23, 2021

ashutosh-b-b commented May 23, 2021

YingboMa commented May 23, 2021

valentinsulzer commented May 23, 2021

ChrisRackauckas commented May 23, 2021

shashi commented May 31, 2021

valentinsulzer commented May 31, 2021

ashutosh-b-b commented Jun 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YingboMa commented Jun 12, 2021

YingboMa commented Jun 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YingboMa commented Jun 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented May 21, 2021 •

edited