Skip to content

Add day only option for calendar embedding#66

Open
smartin98 wants to merge 6 commits intoNVlabs:mainfrom
smartin98:daily_cal_embed
Open

Add day only option for calendar embedding#66
smartin98 wants to merge 6 commits intoNVlabs:mainfrom
smartin98:daily_cal_embed

Conversation

@smartin98
Copy link
Copy Markdown

@nbren12

This just allows for the use case where the data has only daily resolution and second_of_day embedding in the calendar embedding becomes redundant.

embeddings.py:
CalendarEmbeddingOnlyDay is the same as CalendarEmbedding except for returning only the day_of_year channels in the embedding. Still takes second_of_day argument for compatibility but this is not used.

networks.py:
Added flag in SongUNet to enable the OnlyDay embedding, defaults to False.

Comment thread src/cbottle/models/embedding.py Outdated
return torch.concat([a, b], dim=1) # (n c x)

class CalendarEmbeddingOnlyDay(torch.nn.Module):
"""Time embedding assuming 365.25 day years. Only uses day_of_year for use cases with daily resolution, second_of_day arg preserved for compatibility.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

line is too long. Is the linter passing?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

shortened and fixed the linter errors

@nbren12
Copy link
Copy Markdown
Collaborator

nbren12 commented Nov 24, 2025

Needs test coverage:

  1. add a test like this here
    def test_CalendarEmbedding():
  2. add pytest.mark.parametrize for the new option here:
    def test_SongUnetCalendarEmbeddings():

@smartin98
Copy link
Copy Markdown
Author

Needs test coverage:

1. add a test like this here https://github.com/NVlabs/cBottle/blob/4d299faf0fd83135422bfa29a3590655a502084a/tests/unit/test_networks.py#L181

2. add pytest.mark.parametrize for the new option here: https://github.com/NVlabs/cBottle/blob/4d299faf0fd83135422bfa29a3590655a502084a/tests/unit/test_networks.py#L196

added tests

@nbren12
Copy link
Copy Markdown
Collaborator

nbren12 commented Nov 25, 2025

@smartin98 These tests are failing

tests/unit/test_networks.py::test_SongUnetCalendarEmbeddings[False] FAILED [ 89%]
tests/unit/test_networks.py::test_SongUnetCalendarEmbeddings[True] FAILED [ 90%]

@smartin98
Copy link
Copy Markdown
Author

@nbren12 sorry for the delay on this, I fixed the test and rebased to main.

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