Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests #23

Open
Edenhofer opened this issue Feb 16, 2024 · 5 comments
Open

Tests #23

Edenhofer opened this issue Feb 16, 2024 · 5 comments
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@Edenhofer
Copy link

I think it would be great if the repository would include some basic tests of the software as well as, e.g., an end-to-end test of a demonstration script.

Part of openjournals/joss-reviews#6354 .

@kazuakiyama
Copy link
Member

kazuakiyama commented Oct 16, 2024

@Edenhofer
Based on the comments, tutorials are now dynamically compiled in GitHub flow, rather than showing static outputs. The documentation now includes different source structures (GRMHD, and a more simple geometric model) as well. We updated the library so that users can use StableRNG for reproducivility and it has been included in the tutorials. This should work as a fairly good end-to-end test as users can directly compare their local outputs with those in the tutorial.

We will explore the best way to give unit tests in the next few days --- this is the final piece of the revision as the rest of the comments have been all reflected in the library.

@annatartaglia
Copy link
Contributor

@Edenhofer
After the discussion within the team, we have decided to additionally provide the reference processed data in our documentation, which will provide the ultimate end-to-end unit tests to the users. You can find the corresponding documentation pages here:

https://ehtjulia.github.io/ScatteringOptics.jl/v0.1.4/diffractive
https://ehtjulia.github.io/ScatteringOptics.jl/v0.1.4/refractive

As @kazuakiyama wrote, the julia code blocks are executed upon each commit to GitHub, which will automatically keep updating the outputs with respect to the updates in the package. In addition to the plots that @kazuakiyama mentioned, the documentation further produces the corresponding image data and some numerical array for the scattering kernel in the community standard formats FITS and HDF5, respectively. We provide links to those data in the tutorial pages, where users can download and compare with their own outputs. We now include two different types of input models in the tutorial (one provided fits file and one Gaussian model that users can generate themselves using VLBISkyModels.jl), which will also help user tests. Thanks for the helpful suggestions!

@Edenhofer
Copy link
Author

This is great! The documentation is now much more accessible and I can reproduce the demos without issues.

One final remark: I think it is fantastic that the code within the documentation is automatically run as part of the CI but I think it would be great if the essential parts therein would also be proper unit tests such that errors can be flagged automatically.

@kazuakiyama
Copy link
Member

kazuakiyama commented Oct 30, 2024

@Edenhofer
Our decision was in part because this automatic run with CI has a convenient procedure that shows visualization of products as part of the documentation, so we thought it was a more digestible automatic test than more standard run tests. I do agree that implementing run tests is indeed good for reproducibility. Let us discuss what would be the best tests for the package. We will likely implement run tests similar to the documentation following your suggestion.

@Edenhofer
Copy link
Author

I think it is totally fine if the documentation and the test share a lot of code. I'm merely advocating for some form of unit tests :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants