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

Named Layers for Container Types #79

Closed
avik-pal opened this issue Jun 29, 2022 · 3 comments · Fixed by #143
Closed

Named Layers for Container Types #79

avik-pal opened this issue Jun 29, 2022 · 3 comments · Fixed by #143
Labels
enhancement New feature or request

Comments

@avik-pal
Copy link
Member

Currently containers can only take tuples, and they set the named to layer_1, layer_2.... #44 was an initial prototype for this. But we need to do it for all container layers implemented in Lux.

@avik-pal avik-pal added the enhancement New feature or request label Jun 29, 2022
@lungd
Copy link
Contributor

lungd commented Aug 1, 2022

It would be nice if one could solve this issue to get type stable containers.
What was missing in that PR besides applybranching and applypairwisefusion? Maybe I will have some spare time to give it a try the next few days.

@avik-pal
Copy link
Member Author

avik-pal commented Aug 2, 2022

In terms of the functionality, nothing was missing except applybranching, applyparallel, and applypairwisefusion.

I am curious as to what you mean by type stable containers. Currently all the container layers in lux should be type stable

@lungd
Copy link
Contributor

lungd commented Aug 2, 2022

I tried to recreate the MWE I played with some weeks ago. I don't see the performance drop any longer (~20 additional allocations + exec time). Most likely, I actually benchmarked the construction of the chain. That would explain the additional allocations because I see a Body::Any for the construction with the current version vs no red line and 0 allocations with a NamedTuple.

Sorry for the false alarm. Finishing that PR to get rid of the red lines one sees using @code_warntype would be a good idea, though.

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

Successfully merging a pull request may close this issue.

2 participants