JuliaDynamics/DrWatson.jl

produce_or_load doubles filename prefixes

Opened this issue · 5 comments

Describe the bug
When a config variable default_prefix is modified, then produce_or_load saves the output to a file with a name starting with two prefixes instead of one.

Minimal Working Example

using DrWatson
using Dates

quickactivate(@__DIR__)

mutable struct Params
    A::Int 
    B::Float64 
    C::String 
    D::Array
    date::Date
end

function Params(;
    A = 36,
    B = 10.,
    C = "test",
    D = rand([1,2,3,5], 5)
)
    t_ = Dates.now()
    return Params(A, B, C, D, Date(t_))
end

mutable struct Params2
    A2::Int 
    B2::Float64 
    C2::String 
    D2::Array
    date::Date
end

function Params2(;
    A2 = 36,
    B2 = 10.,
    C2 = "test",
    D2 = rand([1,2,3,5], 5)
)
    t_ = Dates.now()
    return Params2(A2, B2, C2, D2, Date(t_))
end


DrWatson.default_prefix(params::Params) = "Experiment_"*string(params.date) 
DrWatson.allaccess(::Params) = (:A, :B, :C, :D)
DrWatson.allaccess(::Params2) = (:A2, :B2, :C2, :D2)


params = Params(A = 36,B = 10., C = "test", D = rand([1,2,3,5], 5))
params2 = Params2(A2 = 36, B2 = 10., C2 = "test", D2 = rand([1,2,3,5], 5))

what_to_save = produce_or_load(projectdir("test", "produce_or_load_test_files"), params; 
    force = false, loadfile = true) do params
        "some test output"
        Dict("A" => rand(5), "B" => rand(1:5, 5))
end

what_to_save = produce_or_load(projectdir("test", "produce_or_load_test_files"), params2; 
    force = false, loadfile = true) do params2
        "some test output"
        Dict("A" => rand(5), "B" => rand(1:5, 5))
end

As a result following files are saved: A2=36_B2=10.0_C2=test.jld2 and Experiment_2023-06-30_Experiment_2023-06-30_A=36_B=10.0_C=test.jld2.
The first file is the result of saving Params2 for which default_prefix was not modified. The second file has the prefix repeated.

Versions
DrWatson 2.12.5
Julia 1.6.4

Proposed solution
I propose moving line 91 in file saving_files.jl to line 87, and changing line 83, so they would be:

    if filename === nothing
        filename = config -> savename(prefix, config, suffix; kwargs...)
    end
    # Prepare absolute file name
    if filename isa AbstractString
        name = append_prefix_suffix(filename, prefix, suffix)
    else
        name = string(filename(config))
    end

The reason for that is that savename function runs default_prefix(c) by itself.

oh shit this looks like a super duper major bug that we have to fix asap. it may break people's code when we fix it but we have to!!! And we have to put a test for it as well!!!!!!!!!!!

@pjgorski your solution is correct! Could you please open a PR?

I think this bug was introduced when we allowed produce_or_load to work with arbitrary naming functions, such as the hash maps. So thankfully it is a recent bug.

This is still (or again, perhaps) broken on v2.13.0. The following MWE will produce both Experiment.jld2 and Experiment_Experiment.jld2. Removing this line may fix this issue. Would doing so introduce any new bugs?

using DrWatson

struct Experiment end

DrWatson.default_prefix(::Experiment) = "Experiment"

filename(c) = savename(c; connector=" ")
produce(::Experiment) = Dict("data" => randn())
produce_or_load(produce, Experiment()) # good
produce_or_load(produce, Experiment(); filename) # bad

@jonas-schulze if you put in a PR together and add a test for it we can see if anything breaks!