search

open-AIMS / ADRIA.jl

ADRIA: Adaptive Dynamic Reef Intervention Algorithms. A multi-criteria decision support platform for informing reef restoration and adaptation interventions.
MIT License
14 stars 5 forks source link

Fix miscellaneous of broken tests #813

Closed Zapiano closed 2 months ago

Zapiano commented 2 months ago

In most cases, it's clear that the tests stopped working because of the changes to adapt ADRIA to use CoralBlox. In some cases it's not clear to me why this was working before. There is one particular case, Aqua.test_stale_deps(ADRIA) is raising an error that I don't understand, so I commented this test until we figure that out.

The error in question:

julia> Aqua.test_stale_deps(ADRIA)
Test Failed at C:\Users\pribeiro\.julia\packages\Aqua\tHrmY\src\stale_deps.jl:31
  Expression: isempty(stale_deps)
   Evaluated: isempty(Base.PkgId[SciMLBase [0bca4576-84f4-4d90-8ffe-ffa030f20462]])

ERROR: There was an error during testing
ConnectedSystems commented 2 months ago

There is one particular case, Aqua.test_stale_deps(ADRIA) is raising an error that I don't understand

It's saying there are dependencies defined in Manifest or Project.toml but are not used. Is this an artifact of SparseArrayKit being removed then added back in?

Zapiano commented 2 months ago

There is one particular case, Aqua.test_stale_deps(ADRIA) is raising an error that I don't understand

It's saying there are dependencies defined in Manifest or Project.toml but are not used. Is this an artifact of SparseArrayKit being removed then added back in?

SparseArrayKit appears as a weak dep. Should it be removed? image

Zapiano commented 2 months ago

There is one particular case, Aqua.test_stale_deps(ADRIA) is raising an error that I don't understand

It's saying there are dependencies defined in Manifest or Project.toml but are not used. Is this an artifact of SparseArrayKit being removed then added back in?

So, when you run Aqua.test_stale_deps, is runs this function Aqua.find_stale_deps and what is does, basically, is list all dependencies and list all loaded dependencies and compare (and, when I run this, I get precisely SciMLBase [0bca4576-84f4-4d90-8ffe-ffa030f20462]).

No, why is SciMLBase not being loaded...?

ConnectedSystems commented 2 months ago

No, why is SciMLBase not being loaded...?

SciMLBase should be a core dependency of some other package developed by the SciML group.

Looking at the git blame log, it was added as a dependency 2 years ago.

From memory, there was some advice from the SciML group to load SciMLBase to reduce precompilation times or something along those lines. But I believe it was to help with DifferentialEquations.jl or OrdinaryDiffEq.jl, both of which we will no longer be using.

So I think safe to remove SciMLBase as a direct dependency. We're not importing it ourselves that I can see either.

Zapiano commented 2 months ago

@ConnectedSystems ready for review.