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

Issue with Running Tests: Need to Add PiCLES Manually #31

Open
Sumanshekhar17 opened this issue Aug 12, 2024 · 13 comments
Open

Issue with Running Tests: Need to Add PiCLES Manually #31

Sumanshekhar17 opened this issue Aug 12, 2024 · 13 comments
Labels
documentation Improvements or additions to documentation

Comments

@Sumanshekhar17
Copy link
Collaborator

Hi @mochell, While following the quick start guide, I encountered an issue when trying to run the T04_2D_reg_test.jl file. Specifically, I had to manually add the PiCLES package in the test repository in order to run the test file, which can be confusing with current documentation.

Steps to Reproduce:

  1. Clone the PiCLES repository as described in the quick start guide.
  2. Navigate to the PiCLES directory.
  3. Activate the environment and install dependencies using:
using Pkg
Pkg.activate(".")
Pkg.instantiate()

Navigate to the tests directory.

Attempt to run the T04_2D_reg_test.jl file using:

include("T04_2D_reg_test.jl")

  Activating project at `~/Desktop/PiCLES/PiCLES/tests/PiCLES`
ERROR: LoadError: ArgumentError: Package PiCLES not found in current path.
- Run `import Pkg; Pkg.add("PiCLES")` to install the PiCLES package.
Stacktrace:
 [1] macro expansion
   @ ./loading.jl:1772 [inlined]
 [2] macro expansion
   @ ./lock.jl:267 [inlined]
 [3] __require(into::Module, mod::Symbol)
   @ Base ./loading.jl:1753
 [4] #invoke_in_world#3
   @ ./essentials.jl:926 [inlined]
 [5] invoke_in_world
   @ ./essentials.jl:923 [inlined]
 [6] require(into::Module, mod::Symbol)
   @ Base ./loading.jl:1746
 [7] include(fname::String)
   @ Base.MainInclude ./client.jl:489
 [8] top-level scope
   @ REPL[2]:1
in expression starting at /Users/ss4338/Desktop/PiCLES/PiCLES/tests/T04_2D_reg_test.jl:9

Workaround
I resolved this by manually adding the PiCLES package while I was in the test repository using:

julia> Pkg.activate(".")
  Activating new project at `~/Desktop/PiCLES/PiCLES/tests`

and then run

include("T04_2D_reg_test.jl")

Additionally, the test case (T04_2D_reg_test.jl) requires the Oceananigans package. This package seems to be missing from the environment, possibly because it's not listed in the Project.toml or Manifest.toml files. Adding Oceananigans manually using Pkg.add("Oceananigans") should resolve the issue.

Can we add a paragraph on why it needs Oceananigans.jl ?

@Sumanshekhar17 Sumanshekhar17 added the documentation Improvements or additions to documentation label Aug 12, 2024
@glwagner
Copy link
Collaborator

You may be interested this tool: https://github.com/JuliaTesting/TestEnv.jl

Not sure it applies here, but test environments can have requirements that are additional to those required strictly by the package.

@Sumanshekhar17
Copy link
Collaborator Author

Sumanshekhar17 commented Aug 12, 2024

Thanks @glwagner .

Also @mochell in the current version, T04_2D_reg_test.jl is missing the import of Revise package. Maybe you want the user to include Revise in the Julia session and that's why not included in your code?

using Revise

Without this the code isn't running!

Here is the error message that I was getting -

julia> include("T04_2D_reg_test.jl")
  Activating project at `~/Desktop/PiCLES/PiCLES/tests/PiCLES`
ERROR: LoadError: UndefVarError: `Revise` not defined
Stacktrace:
 [1] top-level scope
   @ ~/Desktop/PiCLES/PiCLES/tests/T04_2D_reg_test.jl:72
 [2] include(fname::String)
   @ Base.MainInclude ./client.jl:489
 [3] top-level scope
   @ REPL[11]:1
in expression starting at /Users/ss4338/Desktop/PiCLES/PiCLES/tests/T04_2D_reg_test.jl:72


and it was referring to line 72, here is the code where Revise is being used -

# define ODE system and parameters
particle_system = PW.particle_equations(u, v, γ=Const_ID.γ, q=Const_ID.q);

default_ODE_parameters = (r_g=0.85, C_α=Const_Scg.C_alpha,
    C_φ=Const_ID.c_β, C_e=Const_ID.C_e, g=9.81)

Revise.retry()

@glwagner
Copy link
Collaborator

Why would Revise be used in a test? @Sumanshekhar17 can you please link to the code / lines in the code in your comments, its hard to follow along without them.

@Sumanshekhar17
Copy link
Collaborator Author

Sumanshekhar17 commented Aug 12, 2024

Sorry I updated my comment. I don't know why Revise is being used but after including this package the code ran fine!
code snippet from T04_2D_reg_test.jl where I added one line to include this Revise

#using Plots

using Pkg
Pkg.activate("PiCLES/")

import Plots as plt
using Setfield, IfElse
using Revise
using PiCLES.ParticleSystems: particle_waves_v5 as PW

@glwagner
Copy link
Collaborator

Can you paste a hyperlink to the code into this thread?

@glwagner
Copy link
Collaborator

Something like this:

module PiCLES

I just don't know where in the repo the code is you're referring to. It's also helpful to link to individual lines!

@Sumanshekhar17
Copy link
Collaborator Author

Sumanshekhar17 commented Aug 12, 2024

Oh yeah, here is the code -

Revise.retry()

@glwagner
Copy link
Collaborator

Nice thanks. Note when you put a hyperlink to code, if you do not wrap it in text, it will display the code inline which is very helpful for reading a comment quickly. (I have edited your comment to use this feature.) It is also possible to link to a range of lines, eg

# provide function handles for ODE and Simulation in the right format
u(x, y, t) = u_func(x, y, t)
v(x, y, t) = v_func(x, y, t)
winds = (u=u, v=v)
# define ODE system and parameters
particle_system = PW.particle_equations(u, v, γ=Const_ID.γ, q=Const_ID.q);
default_ODE_parameters = (r_g=0.85, C_α=Const_Scg.C_alpha,
C_φ=Const_ID.c_β, C_e=Const_ID.C_e, g=9.81)
Revise.retry()
# Default initial conditions based on timestep and chaeracteristic wind velocity
WindSeamin = FetchRelations.MinimalWindsea(U10, V10, DT )
default_particle = ParticleDefaults(WindSeamin["lne"], WindSeamin["cg_bar_x"], WindSeamin["cg_bar_y"], 0.0, 0.0)

do that you edit the hyperlink. To see the syntax, try to "edit" my post, and you will see how I added a line range to the end of the hyperlink.

@glwagner
Copy link
Collaborator

glwagner commented Aug 12, 2024

As for

Revise.retry()

presumably its irrelevant and can be deleted.

I think there is a broader problem with the tests. In the first place the correct directory name is test --- not tests:

https://github.com/mochell/PiCLES/tree/main/tests

This matters because the tests must be runnable via

using Pkg
Pkg.test()

from the package environment. This is required for CI. This comes from the docstring for Pkg.test:

help?> Pkg.test
  Pkg.test(; kwargs...)
  Pkg.test(pkg::Union{String, Vector{String}; kwargs...)
  Pkg.test(pkgs::Union{PackageSpec, Vector{PackageSpec}}; kwargs...)

  Keyword arguments:

    •  coverage::Bool=false: enable or disable generation of coverage statistics.

    •  allow_reresolve::Bool=true: allow Pkg to reresolve the package versions in the test environment

    •  julia_args::Union{Cmd, Vector{String}}: options to be passed the test process.

    •  test_args::Union{Cmd, Vector{String}}: test arguments (ARGS) available in the test process.

  │ Julia 1.9
  │
  │  allow_reresolve requires at least Julia 1.9.

  Run the tests for package pkg, or for the current project (which thus needs to be a package) if no positional argument is given to Pkg.test. A package
  is tested by running its test/runtests.jl file.

Moreover the tests cannot make any plots nor rely on GLMakie. GLMakie will fail to load on a headless system, like those used for CI:

using GLMakie

I strongly suggest distinguishing between "examples" or "validation cases", and tests, which have a specific meaning in the context of CI.

I also would suggest that before generating documentation about how to run the this cases (some of which perhaps are meant to become tests), that this infrastructure be created. Without CI and testing the package will be difficult to develop I think because many things will break routinely.

@mochell
Copy link
Owner

mochell commented Aug 14, 2024

yes the Revise.retry() is not relevant here, its useful for debugging.

I know @glwagner the tests are on the way, just no time to develop that all the way through.
Look here:
https://github.com/mochell/PiCLES/blob/27-write-unit-tests/test/runtests.jl

So you would exclude validations, i.e. testing if the numbers are right, from the strict tests?

@mochell
Copy link
Owner

mochell commented Aug 14, 2024

Hi @mochell, While following the quick start guide, I encountered an issue when trying to run the T04_2D_reg_test.jl file. Specifically, I had to manually add the PiCLES package in the test repository in order to run the test file, which can be confusing with current documentation.

Workaround I resolved this by manually adding the PiCLES package while I was in the test repository using:

julia> Pkg.activate(".")
  Activating new project at `~/Desktop/PiCLES/PiCLES/tests`

and then run

include("T04_2D_reg_test.jl")

Additionally, the test case (T04_2D_reg_test.jl) requires the Oceananigans package. This package seems to be missing from the environment, possibly because it's not listed in the Project.toml or Manifest.toml files. Adding Oceananigans manually using Pkg.add("Oceananigans") should resolve the issue.

Can we add a paragraph on why it needs Oceananigans.jl ?

thanks for trying this.

  • Yes, we should add Oceananigans.jl then.
  • Running is rn is so clunky because it is not a registered package yet. Once I did that, the Project.toml should be easier to maintain.

@glwagner
Copy link
Collaborator

It doesn't need to be a registered package to make the tests, nor to add CI. Registering the package is purely a convenience for installation.

@mochell
Copy link
Owner

mochell commented Sep 18, 2024

I revised the introduction in the branch main-initital_paper. It should work now. The repository will be public soon.

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
Projects
None yet
Development

No branches or pull requests

3 participants