Skip to content

[WIP]Add flow evaluation functions#19

Merged
zygmuntszpak merged 7 commits intoJuliaImages:masterfrom
Deepank308:master
Apr 17, 2019
Merged

[WIP]Add flow evaluation functions#19
zygmuntszpak merged 7 commits intoJuliaImages:masterfrom
Deepank308:master

Conversation

@Deepank308
Copy link
Copy Markdown
Contributor

Added end point error evaluation function. Currently, the function evaluates error iff ground truth flow is also provided as an array.

@Deepank308
Copy link
Copy Markdown
Contributor Author

Deepank308 commented Mar 9, 2019

@zygmuntszpak, I was searching for the standard methods for evaluations, I found out this. My doubt is : Is it a convention to use .flo file, in a standard widely accepted format(as in here), to store the ground truth flow vector or a user may directly pass the ground truth flow vector? Because I am not able to read from these .flo file in Julia.

I also referred to OpenCV contrib repository for some idea, they also end up using the .flo file for ground truth flow in the above specified format. So, I also tried to read these .flo file in Julia but couldn't, however in C++ I could.

I am still trying to successfully read the .flo files, once it's done will add a commit for the reading function.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 9, 2019

Codecov Report

Merging #19 into master will decrease coverage by 3.17%.
The diff coverage is 50.7%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #19      +/-   ##
==========================================
- Coverage    64.8%   61.62%   -3.18%     
==========================================
  Files           7        7              
  Lines         375      443      +68     
==========================================
+ Hits          243      273      +30     
- Misses        132      170      +38
Impacted Files Coverage Δ
src/ImageTracking.jl 100% <100%> (ø) ⬆️
src/utility.jl 46.57% <48.52%> (-40.93%) ⬇️

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 a83b91f...be59eb4. Read the comment docs.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 9, 2019

Codecov Report

Merging #19 into master will decrease coverage by 1.84%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #19      +/-   ##
==========================================
- Coverage    64.8%   62.95%   -1.85%     
==========================================
  Files           7        7              
  Lines         375      386      +11     
==========================================
  Hits          243      243              
- Misses        132      143      +11
Impacted Files Coverage Δ
src/ImageTracking.jl 100% <ø> (ø) ⬆️
src/utility.jl 36.84% <0%> (-50.66%) ⬇️

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 a83b91f...71af507. Read the comment docs.

@Deepank308
Copy link
Copy Markdown
Contributor Author

@zygmuntszpak I have put the error evaluation functions in the same utility.jl. Or, should I keep seperately the flow visualization function and the error evaluation functions?

@zygmuntszpak
Copy link
Copy Markdown
Member

@zygmuntszpak I have put the error evaluation functions in the same utility.jl. Or, should I keep seperately the flow visualization function and the error evaluation functions?

I reckon you can put it in utility.jl for now.

With regard to the ground truth, as far as I can tell we should allow the user to specify the ground truth flow as Array{SVector{2, Float64}, 2} and then work on a parser that can read the .flo file and convert it to Array{SVector{2, Float64}, 2}.

Comment thread src/utility.jl Outdated
Comment thread src/utility.jl Outdated
Comment thread src/utility.jl Outdated
Comment thread src/utility.jl Outdated
Comment thread src/utility.jl Outdated
@Deepank308
Copy link
Copy Markdown
Contributor Author

@zygmuntszpak I haven't added the test function yet. Once, I am able to parse the .flo files I will add the test.

@Deepank308
Copy link
Copy Markdown
Contributor Author

The last commit adds following :

  1. .flo file reading function
  2. demo for error evaluation
  3. test for error evaluation
  4. cleans demo_franeback.jl - moving the error evaluation part to new demo_error_evaluation.jl file.

I have changed the name of evaluate_error function to evaluate_flow_error because there is an identically named function under test/franeback.jl and test/lucas_kanade.jl which was throwing error while running the tests.

@zygmuntszpak please review and suggest changes.
Thank you

@Deepank308
Copy link
Copy Markdown
Contributor Author

I'm really sorry for being so late! I was drafting my GSoC proposal.

@Deepank308 Deepank308 changed the title [WIP]Add flow evaluation functions Add flow evaluation functions Mar 23, 2019
@Deepank308 Deepank308 changed the title Add flow evaluation functions [WIP]Add flow evaluation functions Mar 23, 2019
@zygmuntszpak
Copy link
Copy Markdown
Member

zygmuntszpak commented Apr 12, 2019

I wonder if we shouldn't define an AbstractCoordinateConvention and then have RasterConvention and CartesianConvention as subtypes instead of using strings x_y and row_col. For example, we currently use strings to indicate the convention convention="row_col".

@Deepank308
Copy link
Copy Markdown
Contributor Author

@zygmuntszpak done

Copy link
Copy Markdown
Member

@zygmuntszpak zygmuntszpak left a comment

Choose a reason for hiding this comment

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

I had a moment to study the code more carefully and have a few more suggestions. After this, I think we can go ahead and merge. Thank you for your patience and effort.

Comment thread src/ImageTracking.jl Outdated
Comment thread src/utility.jl Outdated
Comment thread src/utility.jl Outdated
Comment thread src/utility.jl Outdated
Comment thread src/utility.jl Outdated
Comment thread src/utility.jl Outdated
Comment thread src/utility.jl Outdated
Comment thread src/utility.jl Outdated
Comment thread src/utility.jl Outdated
Comment thread src/utility.jl Outdated
Comment thread src/utility.jl Outdated
@zygmuntszpak
Copy link
Copy Markdown
Member

Are the tests passing for you locally? I'm not sure why all of the builds have failed on Travis and Appveyor.

@Deepank308
Copy link
Copy Markdown
Contributor Author

Deepank308 commented Apr 17, 2019

What I think the error might be is, ImageTracking is not a part of the registered packages under Julia(at least it is not mentioned here). So, while testing it is throwing some error. The Project.toml doesn't has any name field for the package as in here.

@zygmuntszpak
Copy link
Copy Markdown
Member

Ok, can you confirm that the tests pass on your machine? If so, then I will go ahead and merge and then work on a separate pull-request to register this package etc. I just want us to be reasonably confident that we don't commit broken code to the master branch.

@Deepank308
Copy link
Copy Markdown
Contributor Author

Yes, the tests are passing on my local machine.

@zygmuntszpak zygmuntszpak merged commit 7e329ec into JuliaImages:master Apr 17, 2019
Comment thread Manifest.toml
@@ -0,0 +1,21 @@
[[Libdl]]
Copy link
Copy Markdown
Member

@johnnychen94 johnnychen94 Apr 17, 2019

Choose a reason for hiding this comment

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

If I make it right, generally, we don't provide Manifest.toml in the repo
the old julia/CI system uses REQUIRE, but in most cases we still keep it to keep track of Project.toml
the new julia/CI system uses Project.toml

Comment thread Project.toml
@@ -0,0 +1,2 @@
[deps]
Copy link
Copy Markdown
Member

@johnnychen94 johnnychen94 Apr 17, 2019

Choose a reason for hiding this comment

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

The workflow to generate Project.toml is:

  1. ]generate ImageTracking
  2. add packages required for this package
  3. add packages required in test stage
  4. there'll be Project.toml and Manifest.toml generated
  5. create a [extra] section, and move packages required by test stage only in [deps] section to this section
  6. create a [targets] section, and write in format STAGENAME = ["PKG1", "PKG2"], e.g., test = ["Test"]
  7. delete the Manifest.toml and upload Project.toml

see https://julialang.github.io/Pkg.jl/v1/creating-packages/ for more information

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants