remove project path from model hash

dssg / triage

General Purpose Risk Modeling and Prediction Toolkit for Policy and Social Good Problems

Other

185 stars 60 forks source link

remove project path from model hash #830

Closed shaycrk closed 3 years ago

shaycrk commented 3 years ago

Looks like this should be all we have to do to remove the project path from the model hash, but we may want to bump the version when brining this in since the model hashes won't be backwards-compatible with runs from current versions of triage.

shaycrk commented 3 years ago

For what it's worth, if we really wanted to provide backwards compatibility, the two options I can think of would be:

inspect the models table for a matching model and use the existing hash (if we might already be writing something similar for random numbers, could take advantage of that code)
pass a new use_legacy_hash parameter here (however, _model_hash() is only used pretty deep down the chain, so this would require a lot of changes, and if the parameter were passed via experiment config, we'd also want to be careful that it doesn't change the experiment hash)

thcrock commented 3 years ago

It's probably also worth doing at least the YAML upgrade to silence the security message before tagging the major version.

It's probably not necessary to tag a major version right after this, but rather I'd make the next version, whenever it comes, major instead of minor.

shaycrk commented 3 years ago

Was going to drop something in the docs here, but realized I couldn't find a really clear place to do so (unless I missed it, of course). Aside from the release notes when we update the version, is there somewhere we've generally documented changes that break backward compatibility?