additional ad-hoc waveforms (gaussian, wnb, strings) #195
Conversation
|
@eric-moreno is this still under consideration. It looks like these waveforms exist in lalsimulation, could you add unittests against those? |
deepchatterjeeligo
left a comment
There was a problem hiding this comment.
@eric-moreno please add unittest for the new waveforms.
Add unit test
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||
| # But to create an actual Python shape, we do .cpu().item() at the end. | ||
| temp = (9.0 / f_low) / dt / 2.0 # float in Python | ||
| # If you want GPU math, push it to a GPU tensor first: | ||
| temp_t = torch.tensor(temp, device=device, dtype=torch.float32) |
There was a problem hiding this comment.
this seems to be a constant. Why not make it a register_buffer
There was a problem hiding this comment.
Since we’re only calculating the required time-window size, using a torch.Tensor operation is overkill. I will change it to math.floor.
| # If you want GPU math, push it to a GPU tensor first: | ||
| temp_t = torch.tensor(temp, device=device, dtype=torch.float32) | ||
| half_len = torch.floor(temp_t) # GPU-based floor | ||
| half_len_int = int(half_len.cpu().item()) # bring back to Python int |
There was a problem hiding this comment.
We should not have .cpu() in the waveform generator
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
No description provided.