Skip to content

Conversation

odow
Copy link
Member

@odow odow commented Aug 22, 2023

@blegat
Copy link
Member

blegat commented Aug 23, 2023

It seems that having a special case when the index map is trivial in CleverDict could be helpful here if this becomes the bottleneck

@odow
Copy link
Member Author

odow commented Aug 23, 2023

It was only a bottleneck because of the type stability problem. For the ::AbstractVector case, we were passing in a Vector{Any}, then trying to infer the return type, and then casting back to convert(Vector{Any}. Just lots of unnecessary stuff. The recursion also prevented deeply nested expressions.

One general improvement could be that if the CachingOptimizer knows that the mapping is one-to-one, then it could store nothing here:

model_to_optimizer_map::IndexMap
optimizer_to_model_map::IndexMap

and we could add a new overload to make all of these calls a no-op:
map_indices(m.model_to_optimizer_map, func),

@odow odow merged commit ce536b2 into master Aug 24, 2023
@odow odow deleted the od/nl-perf branch August 24, 2023 01:55
@blegat
Copy link
Member

blegat commented Aug 24, 2023

Yes I agree but I would make that logic inside CleverDict so that places that don't support nothing still work

@odow
Copy link
Member Author

odow commented Aug 24, 2023

I don't understand what CleverDict has to do here?

@blegat
Copy link
Member

blegat commented Aug 24, 2023

The IndexMap is holding CleverDict's. CleverDict is already recognizing if the indices of src are contiguous so it seems natural to also recognize when the mapping is trivial. It's be discussed a few time with @joaquimg and I also reporting that it could improve performance in other situations also if we then don't even create the vector holding the mapping since it's trivial.

@odow odow added the Project: next-gen nonlinear support Issues relating to nonlinear support label Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Project: next-gen nonlinear support Issues relating to nonlinear support

Development

Successfully merging this pull request may close these issues.

2 participants