Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Fix #546 #553

Merged
merged 1 commit into from
Apr 8, 2022
Merged

Fix #546 #553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Chemistry/tests/SystemTests/Molecules/MoleculeTestHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace SystemTests.Molecules
public static class Helper
{

private static readonly SHA256Managed hashMethod = new SHA256Managed();
private static readonly SHA256 hashMethod = SHA256.Create();

/// <summary>
/// Returns a seed to use for the test run based on the class
Expand Down
3 changes: 1 addition & 2 deletions MachineLearning/src/Training.qs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ namespace Microsoft.Quantum.MachineLearning {
trainingSchedule : SamplingSchedule,
validationSchedule : SamplingSchedule
) : (SequentialModel, Int) {
mutable bestSoFar = Default<SequentialModel>()
Copy link
Contributor

@cgranade cgranade Apr 7, 2022

Choose a reason for hiding this comment

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

This may be worth an extended discussion. Before deprecating Default, it was somewhat of an idiom to use Default<UdtType>() w/ ItemName <- foo to kind of get named-item initializers instead of relying purely on positional construction. It may be good to have a language feature similar to Rust's {}-style structs as an alternative way of creating UDT values.

w/ Structure <- (Head(models))::Structure;
mutable bestSoFar = SequentialModel((Head(models))::Structure, [], 0.0);
mutable bestValidation = Length(samples) + 1;

let features = Mapped(_Features, samples);
Expand Down
130 changes: 24 additions & 106 deletions MachineLearning/tests/StructureTests.qs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,33 +10,20 @@ namespace Microsoft.Quantum.MachineLearning.Tests {

@Test("QuantumSimulator")
function NQubitsRequiredFact() : Unit {
let model = Default<ML.SequentialModel>()
w/ Structure <- [
ML.ControlledRotation((3, [7, 9]), PauliX, 0),
ML.ControlledRotation((8, []), PauliY, 1)
];
let model = ML.SequentialModel([
ML.ControlledRotation((3, [7, 9]), PauliX, 0),
ML.ControlledRotation((8, []), PauliY, 1)
], [], 0.);
let actual = ML.NQubitsRequired(model);
EqualityFactI(actual, 10, "Wrong output from NQubitsRequired.");
}

function ExampleModel() : ML.SequentialModel {
return Default<ML.SequentialModel>()
w/ Structure <- [
Copy link
Contributor

@cgranade cgranade Apr 7, 2022

Choose a reason for hiding this comment

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

Similarly here, I think the original code was more readable even if less correct... from that perspective, I agree with your change but would like to kick off a discussion on more readable alternatives going forward?

Copy link
Member Author

@msoeken msoeken Apr 7, 2022

Choose a reason for hiding this comment

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

I agree that the original code was more readable. Do you suggest to keep the version using Default?

Copy link
Contributor

@cgranade cgranade Apr 7, 2022

Choose a reason for hiding this comment

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

I think getting rid of Default is worth temporarily having less readable code, so no disagreement with your PR so much as that I want to keep momentum on a longer-term solution, I guess?

Default<ML.ControlledRotation>()
w/ TargetIndex <- 2
w/ ControlIndices <- [0]
w/ Axis <- PauliX
w/ ParameterIndex <- 0,
Default<ML.ControlledRotation>()
w/ TargetIndex <- 0
w/ ControlIndices <- [1, 2]
w/ Axis <- PauliZ
w/ ParameterIndex <- 1
]
w/ Parameters <- [
1.234,
2.345
];
return ML.SequentialModel([
ML.ControlledRotation((2, [0]), PauliX, 0),
ML.ControlledRotation((0, [1, 2]), PauliZ, 1)],
[1.234, 2.345],
0.0);
}

operation ApplyExampleModelManually(register : Qubit[]) : Unit is Adj + Ctl {
Expand Down Expand Up @@ -64,23 +51,9 @@ namespace Microsoft.Quantum.MachineLearning.Tests {
Fact(All(EqualCR, Zipped(
ML.LocalRotationsLayer(3, PauliY),
[
Default<ML.ControlledRotation>()
w/ TargetIndex <- 0
w/ ControlIndices <- []
w/ Axis <- PauliY
w/ ParameterIndex <- 0,

Default<ML.ControlledRotation>()
w/ TargetIndex <- 1
w/ ControlIndices <- []
w/ Axis <- PauliY
w/ ParameterIndex <- 1,

Default<ML.ControlledRotation>()
w/ TargetIndex <- 2
w/ ControlIndices <- []
w/ Axis <- PauliY
w/ ParameterIndex <- 2
ML.ControlledRotation((0, []), PauliY, 0),
ML.ControlledRotation((1, []), PauliY, 1),
ML.ControlledRotation((2, []), PauliY, 2)
]
)), "LocalRotationsLayer returned wrong output.");
}
Expand All @@ -90,23 +63,9 @@ namespace Microsoft.Quantum.MachineLearning.Tests {
Fact(All(EqualCR, Zipped(
ML.PartialRotationsLayer([4, 5, 6], PauliY),
[
Default<ML.ControlledRotation>()
w/ TargetIndex <- 4
w/ ControlIndices <- []
w/ Axis <- PauliY
w/ ParameterIndex <- 0,

Default<ML.ControlledRotation>()
w/ TargetIndex <- 5
w/ ControlIndices <- []
w/ Axis <- PauliY
w/ ParameterIndex <- 1,

Default<ML.ControlledRotation>()
w/ TargetIndex <- 6
w/ ControlIndices <- []
w/ Axis <- PauliY
w/ ParameterIndex <- 2
ML.ControlledRotation((4, []), PauliY, 0),
ML.ControlledRotation((5, []), PauliY, 1),
ML.ControlledRotation((6, []), PauliY, 2)
]
)), "PartialRotationsLayer returned wrong output.");
}
Expand All @@ -116,23 +75,9 @@ namespace Microsoft.Quantum.MachineLearning.Tests {
Fact(All(EqualCR, Zipped(
ML.CyclicEntanglingLayer(3, PauliX, 2),
[
Default<ML.ControlledRotation>()
w/ TargetIndex <- 0
w/ ControlIndices <- [2]
w/ Axis <- PauliX
w/ ParameterIndex <- 0,

Default<ML.ControlledRotation>()
w/ TargetIndex <- 1
w/ ControlIndices <- [0]
w/ Axis <- PauliX
w/ ParameterIndex <- 1,

Default<ML.ControlledRotation>()
w/ TargetIndex <- 2
w/ ControlIndices <- [1]
w/ Axis <- PauliX
w/ ParameterIndex <- 2
ML.ControlledRotation((0, [2]), PauliX, 0),
ML.ControlledRotation((1, [0]), PauliX, 1),
ML.ControlledRotation((2, [1]), PauliX, 2)
]
)), "CyclicEntanglingLayer returned wrong output.");
}
Expand All @@ -141,46 +86,19 @@ namespace Microsoft.Quantum.MachineLearning.Tests {
function CombinedStructureFact() : Unit {
let combined = ML.CombinedStructure([
[
Default<ML.ControlledRotation>()
w/ TargetIndex <- 0
w/ ControlIndices <- [2]
w/ Axis <- PauliX
w/ ParameterIndex <- 0,

Default<ML.ControlledRotation>()
w/ TargetIndex <- 1
w/ ControlIndices <- [0]
w/ Axis <- PauliX
w/ ParameterIndex <- 1
ML.ControlledRotation((0, [2]), PauliX, 0),
ML.ControlledRotation((1, [0]), PauliX, 1)
],
[
Default<ML.ControlledRotation>()
w/ TargetIndex <- 2
w/ ControlIndices <- [1]
w/ Axis <- PauliZ
w/ ParameterIndex <- 0
ML.ControlledRotation((2, [1]), PauliZ, 0)
]
]);
Fact(All(EqualCR, Zipped(
combined,
[
Default<ML.ControlledRotation>()
w/ TargetIndex <- 0
w/ ControlIndices <- [2]
w/ Axis <- PauliX
w/ ParameterIndex <- 0,

Default<ML.ControlledRotation>()
w/ TargetIndex <- 1
w/ ControlIndices <- [0]
w/ Axis <- PauliX
w/ ParameterIndex <- 1,

Default<ML.ControlledRotation>()
w/ TargetIndex <- 2
w/ ControlIndices <- [1]
w/ Axis <- PauliZ
w/ ParameterIndex <- 2
ML.ControlledRotation((0, [2]), PauliX, 0),
ML.ControlledRotation((1, [0]), PauliX, 1),
ML.ControlledRotation((2, [1]), PauliZ, 2)
]
)), "CombinedStructure returned wrong output.");
}
Expand Down
32 changes: 16 additions & 16 deletions Standard/src/Arrays/Arrays.qs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,23 +211,23 @@ namespace Microsoft.Quantum.Arrays {
let nSliced = Length(remove);
let nElements = Length(array);

if nElements - nSliced <= 0 {
return [];
}

//Would be better with sort function
//Or way to add elements to array
mutable arrayKeep = [0, size = nElements];
mutable sliced = [Default<'T>(), size = nElements - nSliced];
mutable arrayKeep = SequenceI(0, nElements - 1);
mutable sliced = [array[0], size = nElements - nSliced];
mutable counter = 0;

for idx in 0 .. nElements - 1 {
set arrayKeep w/= idx <- idx;
}

for idx in 0 .. nSliced - 1 {
set arrayKeep w/= remove[idx] <- -1;
for idx in remove {
set arrayKeep w/= idx <- -1;
}

for idx in 0 .. nElements - 1 {
if (arrayKeep[idx] >= 0) {
set sliced w/= counter <- array[arrayKeep[idx]];
for idx in arrayKeep {
if idx >= 0 {
set sliced w/= counter <- array[idx];
set counter += 1;
}
}
Expand Down Expand Up @@ -331,16 +331,16 @@ namespace Microsoft.Quantum.Arrays {
/// let split = Partitioned([2,2], [1,5,3,7]);
/// ```
function Partitioned<'T>(nElements: Int[], arr: 'T[]) : 'T[][] {
mutable output = [Default<'T[]>(), size = Length(nElements) + 1];
mutable output = [[], size = Length(nElements) + 1];
mutable currIdx = 0;
for idx in IndexRange(nElements) {
if(currIdx + nElements[idx] > Length(arr)) {
if currIdx + nElements[idx] > Length(arr) {
fail "Partitioned argument out of bounds.";
}
set output w/= idx <- arr[currIdx..currIdx + nElements[idx]-1];
set output w/= idx <- arr[currIdx..currIdx + nElements[idx] - 1];
set currIdx = currIdx + nElements[idx];
}
set output w/= Length(nElements) <- arr[currIdx..Length(arr)-1];
set output w/= Length(nElements) <- arr[currIdx..Length(arr) - 1];
return output;
}

Expand Down Expand Up @@ -483,7 +483,7 @@ namespace Microsoft.Quantum.Arrays {
/// TupleArrayAsNestedArray([(2, 3), (4, 5)]);
/// ```
function TupleArrayAsNestedArray<'T>(tupleList : ('T, 'T)[]) : 'T[][] {
mutable newArray = [Default<'T[]>(), size = Length(tupleList)];
mutable newArray = [[], size = Length(tupleList)];
for idx in IndexRange(tupleList) {
let (tupleLeft, tupleRight) = tupleList[idx];
set newArray w/= idx <- [tupleLeft, tupleRight];
Expand Down
9 changes: 7 additions & 2 deletions Standard/src/Arrays/DrawMany.qs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,13 @@ namespace Microsoft.Quantum.Arrays {
/// - Microsoft.Quantum.Canon.Repeat
operation DrawMany<'TInput, 'TOutput>(op : ('TInput => 'TOutput), nSamples : Int, input : 'TInput)
: 'TOutput[] {
mutable outputs = [Default<'TOutput>(), size = nSamples];
for idx in 0..nSamples - 1 {
if nSamples == 0 {
return [];
}

let first = op(input);
mutable outputs = [first, size = nSamples];
for idx in 1..nSamples - 1 {
set outputs w/= idx <- op(input);
}
return outputs;
Expand Down
10 changes: 7 additions & 3 deletions Standard/src/Arrays/Interleaved.qs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,12 @@ namespace Microsoft.Quantum.Arrays {

Fact(lFirst >= lSecond and lFirst - lSecond <= 1, "Array `first` is either of same size as `second`, or has one more element");

return [Default<'T>(), size = lFirst + lSecond]
w/ 0..2..(lFirst + lSecond - 1) <- first
w/ 1..2..(lFirst + lSecond - 1) <- second;
if lFirst == 0 {
return [];
} else {
return [first[0], size = lFirst + lSecond]
w/ 0..2..(lFirst + lSecond - 1) <- first
w/ 1..2..(lFirst + lSecond - 1) <- second;
}
}
}
52 changes: 32 additions & 20 deletions Standard/src/Arrays/Map.qs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,16 @@ namespace Microsoft.Quantum.Arrays {
/// # See Also
/// - Microsoft.Quantum.Arrays.ForEach
function Mapped<'T, 'U> (mapper : ('T -> 'U), array : 'T[]) : 'U[] {
mutable resultArray = [Default<'U>(), size = Length(array)];

for idxElement in IndexRange(array) {
set resultArray w/= idxElement <- mapper(array[idxElement]);
let length = Length(array);
if length == 0 {
return [];
}

return resultArray;
let first = mapper(array[0]);
mutable retval = [first, size = length];
for idx in 1..length - 1 {
set retval w/= idx <- mapper(array[idx]);
}
return retval;
}

/// # Summary
Expand Down Expand Up @@ -80,13 +83,16 @@ namespace Microsoft.Quantum.Arrays {
/// # See Also
/// - Microsoft.Quantum.Arrays.Mapped
function MappedByIndex<'T, 'U> (mapper : ((Int, 'T) -> 'U), array : 'T[]) : 'U[] {
mutable resultArray = [Default<'U>(), size = Length(array)];

for idxElement in IndexRange(array) {
set resultArray w/= idxElement <- mapper(idxElement, array[idxElement]);
let length = Length(array);
if length == 0 {
return [];
}

return resultArray;
let first = mapper(0, array[0]);
mutable retval = [first, size = length];
for idx in 1..length - 1 {
set retval w/= idx <- mapper(idx, array[idx]);
}
return retval;
}

/// # Summary
Expand Down Expand Up @@ -127,10 +133,13 @@ namespace Microsoft.Quantum.Arrays {
let end = RangeEnd(range);
if ((end - start) / step >= 0) {
let nTerms = (end - start) / step + 1;
mutable resultArray = [Default<'T>(), size = nTerms];
let first = mapper(start);
mutable resultArray = [first, size = nTerms];
mutable idxElement = 0;
for elem in range {
set resultArray w/= idxElement <- mapper(elem);
if idxElement != 0 {
set resultArray w/= idxElement <- mapper(elem);
}
set idxElement += 1;
}
return resultArray;
Expand Down Expand Up @@ -220,13 +229,16 @@ namespace Microsoft.Quantum.Arrays {
/// # See Also
/// - Microsoft.Quantum.Arrays.Mapped
operation ForEach<'T, 'U> (action : ('T => 'U), array : 'T[]) : 'U[] {
mutable resultArray = [Default<'U>(), size = Length(array)];

for idxElement in IndexRange(array) {
set resultArray w/= idxElement <- action(array[idxElement]);
let length = Length(array);
if length == 0 {
return [];
}

return resultArray;
let first = action(array[0]);
mutable retval = [first, size = length];
for idx in 1..length - 1 {
set retval w/= idx <- action(array[idx]);
}
return retval;
}

}
Expand Down
3 changes: 2 additions & 1 deletion Standard/src/Arrays/Reductions.qs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ namespace Microsoft.Quantum.Arrays {
/// ```
function CumulativeFolded<'State, 'T>(fn : (('State, 'T) -> 'State), state : 'State, array : 'T[]) : 'State[] {
mutable current = state;
mutable result = [Default<'State>(), size = Length(array)];
// initialize with current, and then overwrite in loop
mutable result = [current, size = Length(array)];
Copy link
Contributor

@cgranade cgranade Apr 7, 2022

Choose a reason for hiding this comment

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

Does this need an early exit if array is empty, or is that implied by size = 0?

Copy link
Member Author

@msoeken msoeken Apr 7, 2022

Choose a reason for hiding this comment

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

That is implied by size = 0 since we do not need to access or compute some element based on the 0 index in this case.

Copy link
Contributor

@cgranade cgranade Apr 7, 2022

Choose a reason for hiding this comment

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

Makes sense, wasn't entirely sure from a readthrough so wanted to double-check. Thanks!


for (i, elem) in Enumerated(array) {
set current = fn(current, elem);
Expand Down
Loading