Skip to content

make zonal open stacks of length < 32 #950

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

asinghvi17
Copy link
Collaborator

not sure if we want this size or not but that's easy to configure

not sure if we want this size or not but that's easy to configure
@asinghvi17 asinghvi17 added enhancement New feature or request performance labels Apr 22, 2025
@asinghvi17 asinghvi17 requested a review from rafaqz April 22, 2025 18:14
@asinghvi17
Copy link
Collaborator Author

asinghvi17 commented Apr 22, 2025

Probably needs some form of test as well? If creating a stack with 800 layers and saving it to NC makes sense I can do that

@rafaqz
Copy link
Owner

rafaqz commented Apr 22, 2025

Hmm I'm going to change how open works pretty soon (non closure version), so we can open more like 300

Yes nc it's just one object so the number of layers doesn't matter as much.

But let's wait and do it properly. We also need to fix the type stability of multy layer stacks, the scaling isn't good currently

@asinghvi17
Copy link
Collaborator Author

Well some code like this will be needed anyway right?

Or do we actually want to change the way this works, and "chunk" the files into groups that we open one at a time? Horrific I know, but if it's for that sweet sweet performance....

@rafaqz
Copy link
Owner

rafaqz commented Apr 23, 2025

I will try and just write it properly soon. Maybe we can merge this as a stopgap but not worth doing anything complicated. Have you tried it with 32 layers already? How is the compile time?

@asinghvi17
Copy link
Collaborator Author

Haven't tried with 32 layers but IIRC that's the recommended limit for static arrays, so I figure it's probably fine here too

@rafaqz
Copy link
Owner

rafaqz commented Apr 23, 2025

Different problem here... It's 32 deep closures!

@asinghvi17
Copy link
Collaborator Author

Ah yeah it would do that...in that case the limit is 5 ish I believe

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

Successfully merging this pull request may close these issues.

2 participants