Skip to content

Conversation

Datseris
Copy link
Member

@Datseris Datseris commented Nov 2, 2022

Alright, so this PR kinda solves my problems I had with really wanting to use produce_or_load, but also having too many, or too complicated, simulation parameters. I guess it is a simple inbetween step until DrWatson has some proper way to manage metadata/configuration "files". The discussion here #316 and the PR #333 are relevant.

My intermediate solution is to allow arbitrary functions in produce_or_load that take as an input the config container, and return a hopefully unique string for it. In my personal workflow I am using objectid, but I haven't tested it extensively to see really how unique or safe its output is. When DrWatsonSim functionality is in, the change in this PR would still allow us to effortlessly integrate DrWatsonSim with produce_or_load.

See the new example added in the Real World Usage for an overview of why this functionality is useful: https://juliadynamics.github.io/DrWatson.jl/previews/PR366/real_world/#produce_or_load-with-hash-codes-1

@Datseris
Copy link
Member Author

Datseris commented Nov 2, 2022

Ah no my dreams are ruined. Seems like this isn't reliable. Here:

julia> objectid(rand(Random.MersenneTwister(4321), 1000))
0xc8f87467e4da680a

julia> objectid(rand(Random.MersenneTwister(4321), 1000))
0x0205f59056f4c6e6

julia> objectid(rand(Random.MersenneTwister(4321), 1000))
0xd49ed4947c3f3987

@sebastianpech this won't work unfortunately. Is my dream of using produce_or_load with practically everything going down the drain...? Bu hu hu.

@Datseris
Copy link
Member Author

Datseris commented Nov 2, 2022

MY DREAM IS SAVED I JUST NEEDED HASH:

julia> hash(rand(Random.MersenneTwister(4321), 1000))
0xc3437f37a6e8d226

julia> hash(rand(Random.MersenneTwister(4321), 1000))
0xc3437f37a6e8d226

julia> hash(rand(Random.MersenneTwister(4321), 1000))
0xc3437f37a6e8d226

@codecov
Copy link

codecov bot commented Nov 2, 2022

Codecov Report

Merging #366 (6832a41) into main (b68ec7c) will increase coverage by 0.06%.
The diff coverage is 81.81%.

❗ Current head 6832a41 differs from pull request most recent head e375075. Consider uploading reports for the commit e375075 to get more accurate results

@@            Coverage Diff             @@
##             main     #366      +/-   ##
==========================================
+ Coverage   91.12%   91.18%   +0.06%     
==========================================
  Files           8        8              
  Lines         732      749      +17     
==========================================
+ Hits          667      683      +16     
- Misses         65       66       +1     
Impacted Files Coverage Δ
src/DrWatson.jl 80.95% <ø> (ø)
src/project_setup.jl 87.57% <ø> (ø)
src/saving_files.jl 91.58% <80.95%> (+0.47%) ⬆️
src/result_collection.jl 98.18% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Datseris
Copy link
Member Author

Datseris commented Nov 2, 2022

This is good to go.

@Datseris
Copy link
Member Author

Datseris commented Nov 3, 2022

Hm, no, this isn't good to go :( From the example here: https://juliadynamics.github.io/DrWatson.jl/previews/PR366/real_world/#produce_or_load-with-hash-codes-1 we see that once I run the code with exactly the same configuration as it has been run before, e.g., config = Dict("x" => rand(Random.MersenneTwister(1234)), "f" => f1), a new file is produced nevertheless. Why...? hash doesn't seem to return the same number even though I give it the same dictionary with exactly the same entries.

@Datseris
Copy link
Member Author

Datseris commented Nov 4, 2022

Alright, update: I've fixed everything here. First of all, you can use hash on functions. But it only utilizes the function name information, which is kinda useless. You cannot use it at all with anonymous functions. I've updated the example to not use functions in general, as it feels like a risky thing to do, and I also added a warning message.

@JonasIsensee
Copy link
Member

This looks good to me!

@Datseris Datseris merged commit 38da401 into main Nov 4, 2022
@Datseris Datseris deleted the objectid branch November 4, 2022 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants