-
Notifications
You must be signed in to change notification settings - Fork 146
Make config chunk_size maximum rather than minimum #227
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
Changes from 4 commits
d10b027
52236b7
476eafc
d89aeae
8bbcb5e
bf1f80e
de89051
40ae604
8880531
a287563
028a48b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,11 @@ | ||
| model_update: | ||
| presets: | ||
| presets: | ||
| - predict | ||
| - low_mem # to use low memory settings | ||
| custom: | ||
| settings: | ||
| memory: | ||
| eval: | ||
| use_cueq_triangle_kernels: true | ||
| use_deepspeed_evo_attention: true | ||
| use_deepspeed_evo_attention: true | ||
| chunk_size: 1024 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,7 @@ def run_model( | |
| reduce_model_size=True, | ||
| use_deepspeed_evo_attention=False, | ||
| use_triton_triangle_kernels=False, | ||
| chunk_size=None, | ||
| ): | ||
| device = "cuda" if torch.cuda.is_available() else "cpu" | ||
|
|
||
|
|
@@ -59,8 +60,12 @@ def run_model( | |
| use_deepspeed_evo_attention | ||
| ) | ||
|
|
||
| if use_triton_triangle_kernels: | ||
| config.settings.memory.eval.use_triton_triangle_kernels = True | ||
| config.settings.memory.eval.use_triton_triangle_kernels = ( | ||
| use_triton_triangle_kernels | ||
| ) | ||
| if chunk_size is not None: | ||
| config.settings.memory.eval.chunk_size = chunk_size | ||
| config.settings.memory.train.chunk_size = chunk_size | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We never really run chunking with training because the activations stack up anyway in the backward pass so it doesnt save you much. I havent actually run this but i think it may fail some assert. Diffusion conditioning at least has a
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah okay i see now you only have a test for eval mode anyway
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we just delete this line anyway since it cant run
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case, shouldn't we remove
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had it there originally because I needed chunking enabled during validation for some large samples but not for training, so it'll pick what to use in the model like: We could fix the assert in diffusion conditioning to match the other modules so it runs: or change model.py to always set it to None for training and not reference the config, i thought it was easier to distinguish in the config though
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh yeah thats a lot of memory. on 80gb gpus with no kernels I can see the chunking take effect with seq lengths > 1500 tokens. I have a really old and messy script for benchmarking inference speed + mem using random_of3_features. I can clean it up and share it, but I have a bit of a backlog this week so I can just send it for testing #213 so I don't block this PR. As you said, this is really just a config change. Btw realistic values are
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perfect, thanks! I think that gives me enough to do testing without being limited by what I can find in the pdb. A standardized script in-repo would be nice, but you don't need to rush to clean up yours 🙂 So those don't vary much with input or scale with n_tok? I found that homoers used significantly less memory at very large n_tok I think due to msa reuse for the shared sequences, but I didn't dig in. I can test for smaller memory caps and the chunk tuner behavior by limiting torch mem_fraction. The only problem is the combinatorial explosion of options. If 80gb is of particular interest (H100?) I can test it as well. The other option is to test fixed chunk sizes and just track memory. I've got a memory snapshotting callback (happy to clean up and upstream if it would be generally useful). The only issues there are that tuning itself can affect peak due to some clones and then diffusion conditioning getting chunk size 2048 is guarded by the tuner getting on (is the diff there worth it?)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's just the max allowable input
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I looked closer and found the issue. One of the homomers I had Claude dig up for me (6R7M-1) is a 40-chain homomer with an MSA depth of only 122. This was throwing things off and I thought it would be an issue with all homomers to a lesser extent, but it's really just this one that is weird. If n_msa gets subsampled to 1024 every recycle, does it actually make a difference if the random input has n_msa=1024 or n_msa=16384?
Oh right, I forgot that I did that already :-D
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No actually it doesn't make a difference, it's only if you want to exercise the subsampling logic which doesn't matter here. |
||
| config.architecture.loss_module.diffusion.chunk_size = 16 | ||
|
|
||
| of3 = OpenFold3AllAtom(config).to(device=device, dtype=dtype) | ||
|
|
@@ -153,6 +158,25 @@ def test_shape_small_fp32(self, model_phase): | |
| use_deepspeed_evo_attention=False, | ||
| ) | ||
|
|
||
| def test_shape_small_chunk_size_one(self): | ||
|
christinaflo marked this conversation as resolved.
|
||
| batch_size = consts.batch_size | ||
| n_token = 18 | ||
| n_msa = 10 | ||
| n_templ = 3 | ||
|
|
||
| self.run_model( | ||
| batch_size=batch_size, | ||
| n_token=n_token, | ||
| n_msa=n_msa, | ||
| n_templ=n_templ, | ||
| dtype=torch.float32, | ||
| train=False, | ||
| reduce_model_size=True, | ||
| use_deepspeed_evo_attention=False, | ||
| use_triton_triangle_kernels=False, | ||
| chunk_size=1, | ||
| ) | ||
|
|
||
| @compare_utils.skip_unless_triton_installed() | ||
| @compare_utils.skip_unless_cuda_available() | ||
| @pytest.mark.parametrize( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.