Skip to content

more fa tests#105

Merged
ncfrey merged 5 commits intomainfrom
n/more-fa-tests
Jun 12, 2025
Merged

more fa tests#105
ncfrey merged 5 commits intomainfrom
n/more-fa-tests

Conversation

@ncfrey
Copy link
Contributor

@ncfrey ncfrey commented Jun 11, 2025

No description provided.

@ncfrey ncfrey temporarily deployed to test.pypi.org June 11, 2025 21:23 — with GitHub Actions Inactive
@ncfrey ncfrey temporarily deployed to test.pypi.org June 11, 2025 21:25 — with GitHub Actions Inactive
@ncfrey ncfrey temporarily deployed to test.pypi.org June 11, 2025 21:28 — with GitHub Actions Inactive
@ncfrey ncfrey requested a review from karinazad June 11, 2025 23:38
)
attn = attn.transpose(1, 2).view(unpad_bs, -1, dim) # b s h d
attn = unpad_input_only(attn, torch.squeeze(attn_mask) == 1)
# Use native unpadded SDPA without padding/unpadding
Copy link
Collaborator

@karinazad karinazad Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this duplicated with lines above (160-180)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah we need to do unpadded SDPA in FlexBertUnpadAttention and BertAlibiUnpadSelfAttention

except (ImportError, NoCredentialsError, ClientError) as e:
pytest.skip(f"S3 bucket 'prescient-lobster' not accessible: {e}")

checkpoint_path = "s3://prescient-lobster/ume/runs/2025-06-11T02-12-16/epoch=0-step=19500-val_loss=1.0225.ckpt"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this might fail in the future if we change the architecture

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how long does this take? when I load checkpoints locally, it's usually +10 mins

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can remove this test? up to you

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call, i'll remove it and keep the test script. it's really slow and brittle

@ncfrey ncfrey marked this pull request as ready for review June 12, 2025 01:05
@ncfrey ncfrey temporarily deployed to test.pypi.org June 12, 2025 01:05 — with GitHub Actions Inactive
@ncfrey ncfrey merged commit 72a229e into main Jun 12, 2025
5 checks passed
@ncfrey ncfrey deleted the n/more-fa-tests branch June 12, 2025 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants