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

Implementation of basic flow of rebase function. #123

Merged
merged 21 commits into from
Feb 12, 2025

Conversation

PranavTupe2000
Copy link
Contributor

@PranavTupe2000 PranavTupe2000 commented Jan 13, 2025

#104

Fixes #116 #117 #118

Summary of changes

This PR contains implementation of a basic transformation prototype to convert an unrolled program into the specified target basis set.

  • The user call to rebase would then change the basis set to one of the supported basis sets in pyqasm.

Copy link
Member

@TheGupta2012 TheGupta2012 left a comment

Choose a reason for hiding this comment

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

Nice work @PranavTupe2000 ! Just some typos, can merge after that

@ryanhill1
Copy link
Member

ryanhill1 commented Jan 27, 2025

@TheGupta2012 I have no idea why it's rendering the entire gate.py file in the diff. It seems to be doing that for all PRs even when no changes have taken place. Very weird. I thought by just accepting it and merging in the changes, whatever they were, in this PR #129, it might fix it. But after merging in those changes from main to this branch, the issue persists and it renders the whole file in the diff again. If you could take a look, that would be great.

@TheGupta2012
Copy link
Member

TheGupta2012 commented Jan 28, 2025

@ryanhill1 apparently, I see a ^M character at the EOL for each line in every file of maps dir -

image

I took the diff between the HEAD of this PR's branch and your commit 9ac826dc.

  • I think when the PR [FEATURE] Refactor maps.py #106 merged, \r\n was used as the line end for changed files, which is seen when changes are being made from windows systems. I believe that @devilkiller-ag had a windows based machine which updated the EOL chars for every file in maps dir and we couldn't detect that change as the diff was over the complete file (due to refactor).

  • Anyone who touched the files in maps from then on, saved EOL chars according to their OS (mac in your case).
    Now, since @PranavTupe2000's is using a windows machine, the changes are showing again and he's trying to undo the changes which you made to maps!

Note 1 : current PR was free of these changes in maps before the merge from main.
Note 2 : PR #121 and #122 do not see these changes now, probably because @arulandu uses a unix based machine (consistent with your OS).

  • The only thing I can't figure out is why is this happening only for maps. If Pranav is also using a windows machine, any file that he touches and saves should be saved with \r\n chars, rendering complete file diffs.

Anyway, the point is that we will need to fix the EOL chars in some config file for black / tox so that all across we are using consistent line endings. Saw that black normalizes all chars to \n or \r\n depending upon the first line.

@ryanhill1
Copy link
Member

ryanhill1 commented Jan 29, 2025

Yeah sorry about @PranavTupe2000, I was trying to fix the formatting for you and didn't realize it would result in the EOL issue. You can always easily revert my commit! I won't touch it anymore 😅. Hopefully Harshit's EOL linter from #130 will allow an easy fix regardless, once merged.

Apologies again, everyone! Thanks for figuring that out @TheGupta2012

@TheGupta2012
Copy link
Member

No worries @ryanhill1 ! @PranavTupe2000 can you rebase with main and adjust the gates.py so that we can merge? Thanks again!

…feature-rebase-mvp

Merging main into feature-rebase-main
@PranavTupe2000
Copy link
Contributor Author

No worries @ryanhill1, Thanks for helping me! Also thanks @TheGupta2012 for handling EOL issue. I will do the required changes.

@TheGupta2012
Copy link
Member

Hi @PranavTupe2000 , the gates.py file still contains the EOL changes. Can you adjust the line ending of the first line of gates.py to \n and then run the black formatter? This should fix the formatting issues

@TheGupta2012 TheGupta2012 merged commit 8cdeca4 into qBraid:main Feb 12, 2025
20 checks passed
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.

Classify basis sets
3 participants