Repository navigation
Faster hashing of periods that hold a time structure - #104
Conversation
Periods are often dictionary keys (e.g. in JuMP containers indexed by periods). Non-bits periods such as StrategicPeriod hashed through the generic object hash; they now hash their identifying fields (numbers, dates, nested periods), leaving out time structures, profiles and fields of type Any. Bits periods keep the default hash. Equality is unchanged and the hashes are consistent with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
JulStraus
left a comment
JulStraus
left a comment
There was a problem hiding this comment.
I have never worked with the hashes myself so far. So I do not know if what i write here does make sense. However, I have a few questions:
-
Couldn't a problem occur with us neglecting the internal time structure in hash comparison? You mention it in the test (l. 2762 ff., hashes are equal, but types) but nowhere else. Wouldn't a change to the hashing not then lead to breaking changes? How is the indexing working here in JuMP, based on hash or instance?
-
Is l.25,
ex = :(hash($(QuoteNode(nameof(x))), h))
not adding the hash of the specific type as well?
-
Did you see speed advantages when you tested it?
trulsf
left a comment
trulsf
left a comment
There was a problem hiding this comment.
Looks good to me.
Regarding 1., I think hashes are only used to find the dictionary bucket and then equality is used for precise lookup. If we had a Dictionary indexed on StrategicPeriod and had elements from a multitude of different time structures, it could be a bit inefficient. But as we usually have indices from the same time structure it should give efficiency gains.
JulStraus
left a comment
JulStraus
left a comment
There was a problem hiding this comment.
LGTM based on the explanation
4 participants
I'm doing a bit of performance work on building models using
TimeStruct, and the repeated hashing on time periods can reduce the model building time up to 7% in some cases if we improve the hashing.