INM-6 / multi-area-model

A large-scale spiking model of the vision-related areas of macaque cortex.
Other
71 stars 48 forks source link

Informative error message for disagreeing hashes #25

Open AlexVanMeegen opened 3 years ago

AlexVanMeegen commented 3 years ago

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.

jarsi commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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')