Refactor QSharp.Core into Foundation + Core #254
Conversation
swernli
requested review from
IrinaYatsenko,
ScottCarda-MS,
bamarsha,
bettinaheim,
cesarzc and
cgranade
swernli
force-pushed
the
swernli/qsharp-base
branch
from
cf93950 to
d19a5bc
Compare
swernli
force-pushed
the
swernli/qsharp-base
branch
2 times, most recently
from
b825b8c to
76db882
Compare
swernli
force-pushed
the
swernli/qsharp-base
branch
from
76db882 to
a54fa33
Compare
cesarzc
left a comment
cesarzc
left a comment
There was a problem hiding this comment.
I liked that Intrinsics.qs is now divided into Assert.qs and Utils.qs!
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| namespace Microsoft.Quantum.Intrinsic { |
There was a problem hiding this comment.
It doesn't have any user-facing impact, but I'm not sure the name "Utils.qs" communicates to me that this source file contains definitions for two intrinsic callables that are classical, by contrast with the quantum intrinsics in Intrinsic.qs.
There was a problem hiding this comment.
I struggled to come up with a name for this, actually... the only clearer option I could think of would be separating them into Message.qs and Random.qs and having only one operation per file, which feels silly to do for body-intrinsic methods where there isn't any complicated implementation. Anyone have any suggestions here?
There was a problem hiding this comment.
You know, the more I think about it the more I like splitting them anyway, even if it does make very small files. This will kind of line up with what I'm imagining later parts of this decomposition work will look like anyway.
|
I just noticed that ClassicalControl.qs opens the Microsoft.Quantum.Intrinsic namespace but doesn't use it. Between that and the fact that there are rewrite dependencies on it, it should get moved out of the Core and into the Foundation. |
bettinaheim
left a comment
bettinaheim
left a comment
There was a problem hiding this comment.
I haven't looked though it yet, but one thing I wanted to point out:
Several files are listed twice in this diff here. Usually that occurs when there are indeed two versions of the file differing in capitalization. Could you please open your branch on a linux machine (only way to fix it - WSL works too), and make sure you only keep the correct file version such that there are no duplications?
bettinaheim
left a comment
bettinaheim
left a comment
There was a problem hiding this comment.
Blocking to ensure the PR doesn't have any accidental file duplications.
swernli
force-pushed
the
swernli/qsharp-base
branch
3 times, most recently
from
3379775 to
22f25a9
Compare
swernli
force-pushed
the
swernli/qsharp-base
branch
from
22f25a9 to
c01e9b5
Compare
|
@bettinaheim I was able to confirm that there are no file duplications. I think the reason it seems like there are is because I moved several files that have both a .qs and .cs version, such as |
* Refactor QSharp.Core into Foundation + Core * Trying to fix sln * Splitting Utils.qs * Moving ClassicalControl.qs * Reverting Intrinsic.I change
This change refactors the Microsoft.Quantum.QSharp.Core project to introduce a new Microsoft.Quantum.QSharp.Foundation project, where Core is build on top of Foundation. This is the first step toward allowing for alternative decomposition packages (see #249). The Foundation project includes those fundamental elements of the language that all decomposition packages rely on, while the implementation that remains in Core is expected to be overridden/replaced with alternative implementation defined in a specific decomposition package. These decomposition packages will be introduced as part of a later change.