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

[c++] qvector initialization from vector of doubles fails for f32 simulator #1670

Closed
4 tasks done
annagrin opened this issue May 13, 2024 · 3 comments
Closed
4 tasks done
Assignees
Labels
bug Something isn't working verify and close
Milestone

Comments

@annagrin
Copy link
Collaborator

annagrin commented May 13, 2024

Required prerequisites

  • Consult the security policy. If reporting a security vulnerability, do not report the bug using this form. Use the process described in the policy to report the issue.
  • Make sure you've read the documentation. Your issue may be addressed there.
  • Search the issue tracker to verify that this hasn't already been reported. +1 or comment there if it has.
  • If possible, make a PR with a failing test to give us a starting point to work on!

Describe the bug

__qpu__ void test(std::vector<double> inState) {
  cudaq::qvector q = inState;
}

int main() {
  std::vector<double> vec{M_SQRT1_2, 0., 0., M_SQRT1_2};
  auto counts = cudaq::sample(test, vec);
  counts.dump();

  printf("size %zu\n", counts.size());
}

Running

Running this example on a f32 simulator results in a runtime failure:

nvq++ --enable-mlir -v from_state.cpp -o temp && ./temp

terminate called after throwing an instance of 'std::runtime_error'
  what():  Invalid user-provided state data. Simulator is FP32 but state data is FP64.
Aborted

MLIR

cudaq-quake -D CUDAQ_SIMULATION_SCALAR_FP32 from_state.cpp |cudaq-opt

module attributes {llvm.data_layout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128", llvm.triple = "x86_64-unknown-linux-gnu", quake.mangled_name_map = {__nvqpp__mlirgen__function_test._Z4testSt6vectorIdSaIdEE = "_Z4testSt6vectorIdSaIdEE"}} {
  func.func @__nvqpp__mlirgen__function_test._Z4testSt6vectorIdSaIdEE(%arg0: !cc.stdvec<f64>) attributes {"cudaq-entrypoint", "cudaq-kernel", no_this} {
    %0 = cc.stdvec_size %arg0 : (!cc.stdvec<f64>) -> i64
    %1 = math.cttz %0 : i64
    %2 = cc.stdvec_data %arg0 : (!cc.stdvec<f64>) -> !cc.ptr<f64>
    %3 = quake.alloca !quake.veq<?>[%1 : i64]
    %4 = quake.init_state %3, %2 : (!quake.veq<?>, !cc.ptr<f64>) -> !quake.veq<?>
    return
  }
  func.func @_Z4testSt6vectorIdSaIdEE(%arg0: !cc.ptr<!cc.struct<{!cc.ptr<f64>, !cc.ptr<f64>, !cc.ptr<f64>}>>) attributes {no_this} {
    return
  }
}

Steps to reproduce the bug

__qpu__ void test(std::vector<double> inState) {
  cudaq::qvector q = inState;
}

int main() {
  std::vector<double> vec{M_SQRT1_2, 0., 0., M_SQRT1_2};
  auto counts = cudaq::sample(test, vec);
  counts.dump();

  printf("size %zu\n", counts.size());
}

nvq++ --enable-mlir -v from_state.cpp -o temp && ./temp

Expected behavior

Qvector constructor should copy and cast the initializer data to the data types matching the simulation precision, so the example should complete successfully

Is this a regression? If it is, put the last known working version (or commit) here.

Not a regression

Environment

  • CUDA Quantum version:
  • Python version:
  • C++ compiler:
  • Operating system:

Suggestions

No response

@schweitzpgi schweitzpgi self-assigned this May 13, 2024
schweitzpgi added a commit to schweitzpgi/cuda-quantum that referenced this issue May 14, 2024
It may be the case that the state vector data is in a precision that
does not match that of the simulator. The runtime library knows
precisely what simulator has been selected and can therefore convert the
state vector to the expected format at runtime. This conversion may be
*expensive* if the state vector is quite long. As a result, the runtime
may want to emit a warning when a conversion path is taken.

The following conversions happen at runtime with this change:
  vector<float> -> vector<complex<float>>
  vector<float> -> vector<complex<double>>
  vector<double> -> vector<complex<float>>
  vector<double> -> vector<complex<double>>
  vector<complex<float>> -> vector<complex<double>>
  vector<complex<double>> -> vector<complex<float>>

If the input is vector<complex<float>> or vector<complex<double>> and
the simulator's precision is a precise match, no conversions or runtime
overhead is incurred.

Fix NVIDIA#1670
schweitzpgi added a commit to schweitzpgi/cuda-quantum that referenced this issue May 15, 2024
It may be the case that the state vector data is in a precision that
does not match that of the simulator. The runtime library knows
precisely what simulator has been selected and can therefore convert the
state vector to the expected format at runtime. This conversion may be
*expensive* if the state vector is quite long. As a result, the runtime
may want to emit a warning when a conversion path is taken.

The following conversions happen at runtime with this change:
  vector<float> -> vector<complex<float>>
  vector<float> -> vector<complex<double>>
  vector<double> -> vector<complex<float>>
  vector<double> -> vector<complex<double>>
  vector<complex<float>> -> vector<complex<double>>
  vector<complex<double>> -> vector<complex<float>>

If the input is vector<complex<float>> or vector<complex<double>> and
the simulator's precision is a precise match, no conversions or runtime
overhead is incurred.

Fix NVIDIA#1670
schweitzpgi added a commit to schweitzpgi/cuda-quantum that referenced this issue May 15, 2024
It may be the case that the state vector data is in a precision that
does not match that of the simulator. The runtime library knows
precisely what simulator has been selected and can therefore convert the
state vector to the expected format at runtime. This conversion may be
*expensive* if the state vector is quite long. As a result, the runtime
may want to emit a warning when a conversion path is taken.

The following conversions happen at runtime with this change:
  vector<float> -> vector<complex<float>>
  vector<float> -> vector<complex<double>>
  vector<double> -> vector<complex<float>>
  vector<double> -> vector<complex<double>>
  vector<complex<float>> -> vector<complex<double>>
  vector<complex<double>> -> vector<complex<float>>

If the input is vector<complex<float>> or vector<complex<double>> and
the simulator's precision is a precise match, no conversions or runtime
overhead is incurred.

Fix NVIDIA#1670
schweitzpgi added a commit to schweitzpgi/cuda-quantum that referenced this issue May 16, 2024
It may be the case that the state vector data is in a precision that
does not match that of the simulator. The runtime library knows
precisely what simulator has been selected and can therefore convert the
state vector to the expected format at runtime. This conversion may be
*expensive* if the state vector is quite long. As a result, the runtime
may want to emit a warning when a conversion path is taken.

The following conversions happen at runtime with this change:
  vector<float> -> vector<complex<float>>
  vector<float> -> vector<complex<double>>
  vector<double> -> vector<complex<float>>
  vector<double> -> vector<complex<double>>
  vector<complex<float>> -> vector<complex<double>>
  vector<complex<double>> -> vector<complex<float>>

If the input is vector<complex<float>> or vector<complex<double>> and
the simulator's precision is a precise match, no conversions or runtime
overhead is incurred.

Fix NVIDIA#1670
schweitzpgi added a commit that referenced this issue May 16, 2024
…1680)

* Fix mismatches between input state and simulation engine precision.

It may be the case that the state vector data is in a precision that
does not match that of the simulator. The runtime library knows
precisely what simulator has been selected and can therefore convert the
state vector to the expected format at runtime. This conversion may be
*expensive* if the state vector is quite long. As a result, the runtime
may want to emit a warning when a conversion path is taken.

The following conversions happen at runtime with this change:
  vector<float> -> vector<complex<float>>
  vector<float> -> vector<complex<double>>
  vector<double> -> vector<complex<float>>
  vector<double> -> vector<complex<double>>
  vector<complex<float>> -> vector<complex<double>>
  vector<complex<double>> -> vector<complex<float>>

If the input is vector<complex<float>> or vector<complex<double>> and
the simulator's precision is a precise match, no conversions or runtime
overhead is incurred.

Fix #1670

* Remove c++20 code. Wheel builder was choking on it.

* Add test from issue.
@annagrin
Copy link
Collaborator Author

annagrin commented Jun 3, 2024

Update: looks like the code generates unexpected results now, after the PRs above:

{ 00:1000 }
size 1 

Expected: 2

@schweitzpgi
Copy link
Collaborator

@annagrin Is this still an issue or can it be closed?

@bettinaheim bettinaheim added the bug Something isn't working label Jul 1, 2024
@bettinaheim bettinaheim added this to the release 0.8.0 milestone Jul 1, 2024
@bmhowe23
Copy link
Collaborator

This looks like it is working correctly to me now. The above example now returns:

{ 11:495 00:505 }
size 2

Any objections to closing this @annagrin?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working verify and close
Projects
None yet
Development

No branches or pull requests

4 participants