search

openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
704 stars 37 forks source link

[REVIEW]: OmniTrax: A deep learning-driven multi-animal 1 tracking and pose-estimation add-on for Blender #5549

Closed editorialbot closed 6 months ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@FabianPlum<!--end-author-handle-- (Fabian Plum) Repository: https://github.com/FabianPlum/OmniTrax Branch with paper.md (empty if default branch): Version: V_1.0.0 Editor: !--editor-->@Kevin-Mattheus-Moerman<!--end-editor-- Reviewers: @lucasmiranda42, @sfmig, @rizarae-p Archive: 10.5281/zenodo.10817891

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/48a3822e63785944c2014ea7696037c3"><img src="https://joss.theoj.org/papers/48a3822e63785944c2014ea7696037c3/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/48a3822e63785944c2014ea7696037c3/status.svg)](https://joss.theoj.org/papers/48a3822e63785944c2014ea7696037c3)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@lucasmiranda42 & @jonmatthis, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @Kevin-Mattheus-Moerman know.

✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨

Checklists

📝 Checklist for @lucasmiranda42

📝 Checklist for @jonmatthis

📝 Checklist for @sfmig

📝 Checklist for @rizarae-p

editorialbot commented 1 year ago

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

For a list of things I can do to help you, just type:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf
editorialbot commented 1 year ago 
Software report:

github.com/AlDanial/cloc v 1.88  T=0.10 s (448.1 files/s, 169935.6 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
SVG                              5              0             40           9176
Python                          20            995            833           3639
Markdown                         9            256              0            695
Jupyter Notebook                 1              0            292            204
TeX                              1              6              0             67
YAML                             2             12             14             46
XML                              5              0              0             32
-------------------------------------------------------------------------------
SUM:                            43           1269           1179          13859
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 1 year ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.48550/ARXIV.2004.10934 is OK
- 10.1038/s41593-018-0209-y is OK
- 10.7554/eLife.61909 is OK
- 10.7717/peerj.11155 is OK
- 10.1101/2023.04.20.537685 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 1 year ago

Wordcount for paper.md is 538

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

lucasmiranda42 commented 1 year ago

Review checklist for @lucasmiranda42

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Kevin-Mattheus-Moerman commented 1 year ago

@lucasmiranda42, @jonmatthis thanks for the help with this review. I see @lucasmiranda42 has started reviewing. @jonmatthis are you able to get started soon too?

Kevin-Mattheus-Moerman commented 1 year ago

:wave: @jonmatthis

FabianPlum commented 1 year ago

Hi all! Thanks for your help on the review!

Just FYI, I recently fixed a few minor bugs relating to masking and path handling issues a user kindly reported. I now added an updated release that includes these fixes: OmniTrax Version 0.2.3

All the best Fabi

FabianPlum commented 1 year ago

Hi @Kevin-Mattheus-Moerman Is there anything I can/need to do at the moment regarding the revision process?

All the best Fabi

Kevin-Mattheus-Moerman commented 1 year ago

@FabianPlum no I am afraid not. I believe we are waiting for the reviewers to continue. I am going to ping them a reminder again now. Apologies for the delay. I am also afraid that many people may take time off in the summer... so hopefully they can pick this up again soon.

Kevin-Mattheus-Moerman commented 1 year ago

:wave: @lucasmiranda42, @jonmatthis could you please provide an update on this review? Thanks!

Kevin-Mattheus-Moerman commented 1 year ago

@jonmatthis could you please start the review?

lucasmiranda42 commented 1 year ago

Dear @Kevin-Mattheus-Moerman, dear @FabianPlum,

Apologies if it's taking long, but I should be finished with my review before/by next weekend.

Best, and thank you! Lucas

FabianPlum commented 1 year ago

Hi both, thanks so much for your reply! I don't mean to rush anyone, I was just genuinely wondering if I had missed a step in the process that prevented it from continuing.

All the best and I hope you are enjoying the summer! Fabi

lucasmiranda42 commented 1 year ago

No worries, @FabianPlum! Thanks, and same over there! Lucas

Kevin-Mattheus-Moerman commented 1 year ago

:wave: @jonmatthis could you please start the review?

jonmatthis commented 1 year ago

Sure, I'm just passing a bunch of travel and obligations, so I've got some time.

@FabianPlum - would you mind if I did my review in the form a live streamed "first time seeing this" code review on twitch and/or youtube?

You're welcome to join if you want, but I won't be scheduling a time - I'll just pop i when it feels right and opst the link here when I'm doe (along with the standard written reviews)

FabianPlum commented 1 year ago

@jonmatthis - Sure, sounds exciting! Have never seen that type of review before. Either YouTube or Twitch are fine and I'd love to join the stream if you can give me a heads up.

All the best Fabian

Kevin-Mattheus-Moerman commented 1 year ago

@jonmatthis doing a live stream of the review is up to you and fine (/interesting). However, I would suggest you do it independently of the author @FabianPlum. So, @FabianPlum, please do not join the live stream, it is important that all reviewers are able to work on and review the software independently. Thanks.

FabianPlum commented 1 year ago

@Kevin-Mattheus-Moerman okay, no problem. This is my first time submitting to JOSS so I don't know what review practices are common

Kevin-Mattheus-Moerman commented 1 year ago

@lucasmiranda42, @jonmatthis can you please report on review progress? @jonmatthis could you create your checklist by calling @editorialbot generate my checklist?

lucasmiranda42 commented 1 year ago

Dear all,

apologies for the delay. I finish reading the paper and commenting checking what I could, but so far I didn't manage to install the package due to OS incompatibilities (I didn't have access to a Windows machine, and I was aiming to test it using a Parallels VM in MacOS, which unfortunately did not work). I just borrowed a Windows laptop that has a GPU, and I intend to test installation on that machine. I'll keep you posted!

Best, Lucas

FabianPlum commented 1 year ago

Hi all!

I hope you're all well! I just wanted to carefully ask if you have started to have a look at the git repo and functionality or if there are any immediate installation issues that prevent you from conducting parts of the review.

Lemme know if there is anything I can help with!

All the best Fabian

lucasmiranda42 commented 1 year ago

Dear @FabianPlum, dear all,

After testing on several machines I can confirm I managed to successfully install the package! Issues will shortly follow :)

Best, and apologies for the massive delays, Lucas

FabianPlum commented 1 year ago

Thanks so much for the kind feedback @lucasmiranda42! I added a short contributing guide and will have a look if I can make a basic CPU version of OmniTrax run on Ubuntu. As I don't have access to any MacOS systems, I can't really promise a working and tested implementation on that side.

All the best Fabian

Kevin-Mattheus-Moerman commented 1 year ago

@jonmatthis how are you getting on with the review? Could you create your checklist by calling @editorialbot generate my checklist?

Kevin-Mattheus-Moerman commented 11 months ago

@jonmatthis are you able to continue the review and provide an update? Thanks!

Kevin-Mattheus-Moerman commented 11 months ago

@lucasmiranda42 it looks like you have some unticked boxes remaining. Can you help summarise what the remaining issues are from your end? Thanks!

lucasmiranda42 commented 11 months ago

Dear @Kevin-Mattheus-Moerman, dear all,

thank you for your comment. My only concern at the moment is the open issue regarding OS compatibility. If this is too hard to accomplish, a justification why would also suffice on my end. Other than that (which I encompassed under the functionality box) this has green light on my end :)

Best! Lucas

FabianPlum commented 11 months ago

Hi @lucasmiranda42,

Thanks so much for your feedback! I'm in the process of writing up my PhD thesis so I'm a little slow to implement new functionality at the moment. One issue with getting this to run quickly under Linux lies in the way the neural network side to run YOLO is easier to compile for windows where I built the basic libraries that seem to run well on all tested systems. On the general Linux side I'd have to spend more time reading into how this could be built on the fly (or solved differently altogether) as the system landscape is much more heterogeneous.

I will look into this again the second half of this week!

Thanks for all your time and help with the review.

All the best Fabi

lucasmiranda42 commented 11 months ago

Great, @FabianPlum! I'll test the new implementation ASAP. After this we should be good to go on my side. All the best with the thesis!

FabianPlum commented 11 months ago

@lucasmiranda42 Awesome, thanks a lot!

Kevin-Mattheus-Moerman commented 11 months ago

@jonmatthis please let us know if you are still able to review this submission. If you can no longer we'll have to recruit an alternative reviewer. If you are able to still help, it would be great if you could get started at this point, thanks.

jonmatthis commented 11 months ago

Sorry for the delays - I can take a look this week.

Kevin-Mattheus-Moerman commented 11 months ago

That is great, thanks @jonmatthis !

FabianPlum commented 10 months ago

Hi @jonmatthis and @lucasmiranda42

Did you find any more issues that need fixing?

All the best and have a great weekend!

Fabi

lucasmiranda42 commented 10 months ago

Dear all,

happy to say that the issues I presented last time were successfully addressed. All set on my side :)

Best, Lucas

Kevin-Mattheus-Moerman commented 10 months ago

@jonmatthis Are you able to call @editorialbot generate my checklist to get started? Thanks!

jonmatthis commented 10 months ago

Review checklist for @jonmatthis

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

FabianPlum commented 10 months ago

Hi @jonmatthis! Are there any issues with the repo or paper that need to be addressed before you can continue your review? I really do not want to be impolite or seem impatient, but the paper has been in review for over 4 months at this point.

Please let me know if there is anything I can do to expedite the process.

All the best Fabi

jonmatthis commented 10 months ago

Got it running on Blender, but there's a filepath error when I try to run it:

image

image

image

Error:

Running inference on:  [LogicalDevice(name='/device:CPU:0', device_type='CPU'), LogicalDevice(name='/device:GPU:0', device_type='GPU')]

INFO: Initialised darknet network found!

Traceback (most recent call last):
  File "C:\Users\jonma\AppData\Roaming\Blender Foundation\Blender\3.3\scripts\addons\omni_trax\__init__.py", line 132, in execute
    video = bpy.path.abspath(clip.filepath)
AttributeError: 'NoneType' object has no attribute 'filepath'
Error: Python: Traceback (most recent call last):
  File "C:\Users\jonma\AppData\Roaming\Blender Foundation\Blender\3.3\scripts\addons\omni_trax\__init__.py", line 132, in execute
    video = bpy.path.abspath(clip.filepath)
AttributeError: 'NoneType' object has no attribute 'filepath'
jonmatthis commented 10 months ago

Using the data from:

https://drive.google.com/drive/folders/1PSseMeClcYIe9dcYG-JaOD2CzYceiWdl

FabianPlum commented 10 months ago

Thanks a lot! Will look into this right away!

FabianPlum commented 10 months ago

Hi @jonmatthis ! Could you send me the video you are running the tracking on? I can only reproduce the error when I have not loaded any video

jonmatthis commented 10 months ago

Blender crash without error message on clicking TRACK button

Please specify which combination of video and cfg etc settings is expected to work properly

FabianPlum commented 10 months ago

Your configuration looks good, but in the screenshots above it appears you haven't opened any video in your project. With your configuration above, everything should work as expected with any of the example videos provided, e.g: example_ant_recording.mp4

jonmatthis commented 10 months ago

Cool thanks -

In general, your intro tutorial should not have choices in it

You have a few spots where you're like "Here's a bunch of videos/config files - pick one and put it in" - that's not great because then A - You don't know what the user is going to do and B - the user doesn't know what to do!

The consequence of that confusion is the conversation we just had.

You should update your tutorial to specify exactly which files should be input and then exactly what the expected output should be.

FabianPlum commented 10 months ago

That's really good advice, thanks. I'll update the docs right away

jonmatthis commented 10 months ago

Yeah, the implicit 'agreement' of an intro tutorial is "If you do exactly these steps, it should work"

It makes it much cognitively easier for the user (I don't have enough context to make decisions yet!) and MUCH easier for you to field complaints (the vast majority of which will be of the form "I tried it and it didn't work," bless em)

when your intro tutorial is locked in, it minimizes the number of ways I can screw it up , which makes it harder for you to tell if the error is because your code is broken vs a standard PEBKAC error

If you haven't seen it, I highly recommend checking out https://diataxis.fr for a great framework for making good docs/tutorials etc