Skip to content
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

fix(pdsch) #67

Merged
merged 1 commit into from
Nov 3, 2023
Merged

fix(pdsch) #67

merged 1 commit into from
Nov 3, 2023

Conversation

vborchsh
Copy link
Contributor

@vborchsh vborchsh commented Nov 3, 2023

there was missing carrier argument in nrPDSCHDMRSIndices. Aligned with MATLAB defaults. Also added MATLAB default value to DMRSLength. Closes #66.

@vborchsh vborchsh marked this pull request as ready for review November 3, 2023 09:11
@vborchsh vborchsh marked this pull request as draft November 3, 2023 09:12
@catkira
Copy link
Owner

catkira commented Nov 3, 2023

tests fail

…es`. Aligned with MATLAB defaults. Also added MATLAB default value to `DMRSLength`. Closes catkira#66.
@catkira
Copy link
Owner

catkira commented Nov 3, 2023

we should try to minimize API changes, or only do them with major version changes. But it's ok now, because not so many people are using it yet I guess :)

@catkira
Copy link
Owner

catkira commented Nov 3, 2023

until 1.0 everything is experimental :)

@vborchsh
Copy link
Contributor Author

vborchsh commented Nov 3, 2023

we should try to minimize API changes, or only do them with major version changes. But it's ok now, because not so many people are using it yet I guess :)

I've been tying to keep everything the same way as MATLAB. And MATLAB has strict arguments order. So, new features needs to follow MATLAB, or we can align API to our needs?

(I'd keep MATLAB-style for everything)

@catkira
Copy link
Owner

catkira commented Nov 3, 2023

yes keep it Matlab compatible

@catkira catkira marked this pull request as ready for review November 3, 2023 09:33
@catkira catkira merged commit fa4ad9c into catkira:master Nov 3, 2023
5 checks passed
@vborchsh vborchsh deleted the issue/66 branch November 3, 2023 11:12
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.

nrPDSCHDMRSIndices() throws error
2 participants