tldr-group / taufactor

TauFactor is a parallelised solver for calculating tortuosity factors from voxel data.
MIT License
29 stars 6 forks source link

Joss review, reproducibility of performance claims #74

Closed alexsquires closed 1 year ago

alexsquires commented 1 year ago

With regards to a number of points on the reviewers checklist for the JOSS submission:

Performance claims are made in the paper and the data is presented. It looks impressive, but unless I have missed it, there are no scripts or example data to attempt to reproduce. If these could be provided, I would happily tick these all off!

isaacsquires commented 1 year ago

Hi Alex,

Thanks for raising this. This is a tricky one, as the tools that we compare TauFactor to require different environments and installations. It is only PoreSpy that we compare to that is a Python based tool. Having a single executable script to reproduce the results is challenging, as it will depend from machine to machine. Therefore, in the paper, we instead just reference the tools and report our environment and machine configuration.

For reproducibility, I can direct you to the installation guides for each of the tools:

Our reported results were from a single machine with the same example dataset, which is provided in the GitHub repo.

I hope that clears it up!

alexsquires commented 1 year ago

Thank you for the context.

I will try running the example data in these codes, and will tick these off the checklist assuming I can (approximately) verify the performance claims

alexsquires commented 1 year ago

Hi @isaacsquires

I'm having no issues running taufactor, and the speeds certainly seem to be on the order of those reported in the paper, I am, however, not certain I am running like-for-like analysis in porespy.

Would you be able to provide the script you used to carry out your porespy speed test, or failing that, a analogue of the taufactor example you have provided for porespy?

isaacsquires commented 1 year ago

Hi @alexsquires

Thanks for pointing this out. When rerunning the porespy analysis, I found some dependencies have been updated and it now runs much quicker (and so does TauFactor since the new Pytorch update). I have attached the scripts used to run the time comparisons for the most up to date porespy and taufactor: comparison_scripts.zip.

The new porespy times and updated taufactor times have been added to the graph in #75

alexsquires commented 1 year ago

Brilliant, thanks!