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

Informative error message for disagreeing hashes #25

Open
AlexVanMeegen opened this issue Dec 7, 2020 · 3 comments
Open

Informative error message for disagreeing hashes #25

AlexVanMeegen opened this issue Dec 7, 2020 · 3 comments
Assignees

Comments

@AlexVanMeegen
Copy link
Member

As pointed out by @golosio, the file not found error caused by a mismatch in hashes between simulation and analysis is not helpful for the user. This has to be handled properly.

Maybe related: check the codebase before runtime for changes by the user. Doing this properly would need more thought.

@AlexVanMeegen AlexVanMeegen self-assigned this Dec 7, 2020
@AlexVanMeegen AlexVanMeegen changed the title Informative error message for disagreeeing hashes Informative error message for disagreeing hashes Dec 7, 2020
@jarsi
Copy link
Collaborator

jarsi commented Dec 7, 2020

I agree. In general we should highlight the manual updating of sim_params and network_params instead of directly changing the variables in default_params in the readme.

In the other thread (#24) you suggested manually checking for a clean git repo. But wouldn't this just lead to the user commiting a "dirty" default_params file? I suggested comparing the content of default_params against a pre-calculated hash. But this would render everything quite unflexible. Adding new parameters would require recalculating this hash. We could make the default_params file read-only. This would be a small extra barrier the user would have to take for changing the content and maybe make the user question whether this is the correct way of doing things. But I am not really happy with any of those solutions.

@AlexVanMeegen
Copy link
Member Author

I agree with your analysis, and I am also not quite convinced by any of the solutions. Gotta think about this a bit more.

Regarding git: if one also stores the commit used for the simulation, one knows at least exactly which code was used for the simulation. But that's also starting to enter the domain of sumatra..

@mschmidt87
Copy link
Collaborator

If I may chime in: I would not go down the road of checking the state of the repository. As @AlexVanMeegen said, you will end up reimplementing something like sumatra which should be beyond the scope of this code. Rather than, I would suggest to point out that the default parameters should never be changed. In addition, impementing the error message suggested by @golosio in #24 :

self.label = dicthash.generate_hash_from_dict({'params': self.params,
                                                                        'network_label': self.network.label})
if not isinstance(sim_spec, dict)) and sim_spec!=self.label ):
    raise ValueError('The configuration that produced the input simulation label does not match the current configuration')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants