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

38 fix types in the ast #47

Merged
merged 3 commits into from
Jan 15, 2024
Merged

38 fix types in the ast #47

merged 3 commits into from
Jan 15, 2024

Conversation

pablolh
Copy link
Contributor

@pablolh pablolh commented Dec 13, 2023

Changes

  • Implement ControlledSemantic to be able to express CNOT as a
    controlled X, for instance
  • Separate validation of qubit ranges from type-checking in a separate
    ANTLR tree visitor
  • snake_case instead of camelCase for variable names
  • Add docstrings to parsing classes
  • The GateLibrary superclass is used to factorize code for gateset-aware
    classes (e.g. TypeChecker)
  • Semantic classes are removed, absorbed into Gate subclasses
  • Move ALL ANTLR parsing classes to parsing folder - this has already
    been done, but only partially
  • Gates are specified directly with Python functions and a decorator
  • Lay the basis of handling of anonymous gates
  • Writer turned into impl pattern
  • Rename AST -> IR
  • Make test_testinterpreter a proper unit test, before it was testing
    parsing too
  • Visitor pattern for the SquirrelIR class
  • Use those visitors in e.g. writer and test_interpreter
  • Various code fixes
  • Use inspect to know Python function signatures
    This makes the gate semantic API nice to use from Python.

@pablolh pablolh linked an issue Dec 13, 2023 that may be closed by this pull request
@elenbaasc elenbaasc self-requested a review December 13, 2023 16:24
@pablolh pablolh force-pushed the 38-fix-types-in-the-ast branch 3 times, most recently from 9d33eaf to aadb230 Compare December 14, 2023 10:10
@pablolh pablolh force-pushed the 38-fix-types-in-the-ast branch 6 times, most recently from 5c281bd to c4e59db Compare December 15, 2023 12:27
Copy link
Collaborator

@elenbaasc elenbaasc left a comment

Choose a reason for hiding this comment

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

I have been through the code, but mostly to check if there are no weird things or things going wrong. I haven't been able to check/optimize the functionalities. It's quite a big refactor and a really nice one as well! I would still need some time to fully understand how everything links together, but our meeting this morning helped a lot.

The test coverage is now 82%, it would be good if we could increase this (perhaps this could be an issue of its own though).

opensquirrel/circuit.py Outdated Show resolved Hide resolved
opensquirrel/squirrel_ir.py Outdated Show resolved Hide resolved
docs/reference.md Outdated Show resolved Hide resolved
docs/reference.md Outdated Show resolved Hide resolved
docs/reference.md Outdated Show resolved Hide resolved
opensquirrel/writer.py Outdated Show resolved Hide resolved
opensquirrel/writer.py Outdated Show resolved Hide resolved
opensquirrel/writer.py Outdated Show resolved Hide resolved
test/test_replacer.py Outdated Show resolved Hide resolved
test/test_testinterpreter.py Outdated Show resolved Hide resolved
@pablolh
Copy link
Contributor Author

pablolh commented Dec 15, 2023

Thanks Chris for the review, will look asap

@pablolh
Copy link
Contributor Author

pablolh commented Dec 19, 2023

I've tried increasing the test coverage a bit, by looking at what was not covered
Most of the non-covered code is now generated code from ANTLR... Not sure if I can do much about it, the test_parsing.py is already pretty complete

@elenbaasc
Copy link
Collaborator

I've tried increasing the test coverage a bit, by looking at what was not covered Most of the non-covered code is now generated code from ANTLR... Not sure if I can do much about it, the test_parsing.py is already pretty complete

Yes, I see, good point. We could exclude the generated files by adding them to the omit list of the coverage tool in the pyproject.toml:

[tool.coverage.run]
branch = true
omit = ["tests/*", "**/.tox/**","opensquirrel/parsing/antlr/*"]

opensquirrel/parsing/antlr/type_checker.py Outdated Show resolved Hide resolved
opensquirrel/parsing/antlr/type_checker.py Outdated Show resolved Hide resolved
opensquirrel/parsing/antlr/type_checker.py Outdated Show resolved Hide resolved
opensquirrel/squirrel_ir.py Show resolved Hide resolved
opensquirrel/squirrel_ir.py Outdated Show resolved Hide resolved
scripts/gen_reference_page.py Outdated Show resolved Hide resolved
test/test_mckay_decomposer.py Outdated Show resolved Hide resolved
test/test_mckay_decomposer.py Outdated Show resolved Hide resolved
test/test_mckay_decomposer.py Outdated Show resolved Hide resolved
- Implement ControlledSemantic to be able to express CNOT as a
  controlled X, for instance
- Separate validation of qubit ranges from type-checking in a separate
  ANTLR tree visitor
- snake_case instead of camelCase for variable names
- Add docstrings to parsing classes
- The GateLibrary superclass is used to factorize code for gateset-aware
classes (e.g. TypeChecker)
- Semantic classes are removed, absorbed into Gate subclasses
- Move ALL ANTLR parsing classes to parsing folder - this has already
  been done, but only partially
- Gates are specified directly with Python functions and a decorator
- Lay the basis of handling of anonymous gates
- Writer turned into impl pattern
- Rename AST -> IR
- Make test_testinterpreter a proper unit test, before it was testing
  parsing too
- Visitor pattern for the SquirrelIR class
- Use those visitors in e.g. writer and test_interpreter
- Various code fixes
- Use inspect to know Python function signatures
 This makes the gate semantic API nice to use from Python.
@pablolh pablolh merged commit b8b711f into develop Jan 15, 2024
17 checks passed
@elenbaasc elenbaasc deleted the 38-fix-types-in-the-ast branch April 9, 2024 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants