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

produce_or_load doubles filename prefixes #392

Open
pjgorski opened this issue Jun 30, 2023 · 5 comments · May be fixed by #400
Open

produce_or_load doubles filename prefixes #392

pjgorski opened this issue Jun 30, 2023 · 5 comments · May be fixed by #400
Labels
bug Something isn't working high priority It is crucial to resolve this as soon as possible

Comments

@pjgorski
Copy link
Contributor

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.

@Datseris Datseris added bug Something isn't working high priority It is crucial to resolve this as soon as possible labels Jun 30, 2023
@Datseris
Copy link
Member

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!!!!!!!!!!!

@Datseris
Copy link
Member

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

@Datseris
Copy link
Member

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.

@jonas-schulze
Copy link
Contributor

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

@Datseris
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high priority It is crucial to resolve this as soon as possible
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants