QFxp: use integer values for classical sim instead of fxpmath.Fxp

[anurudhp]

• Use raw integer values for simulating QFxp
• Move fxpmath based functionality in QFxp to private functions

To pass inputs to the classical simulator, one should use QFxp.to_fixed_width_int to convert a float to the correct int for simulation.

(part of #1142)

In a follow-up I will remove the classical_sim.py ints_to_bits and bits_to_ints and update all bloqs to use qdtype features.

[fdmalone]

What's the motivation here? Is it faster?

[anurudhp]

What's the motivation here? Is it faster?

Yeah, Fxp is some quite slow, about 2-5x. And fxpmath is quite unstable, it does not correctly preserve behaviour for truncation etc. I ran into a lot of issues in #1142.

The current simulator uses ints for QFxp as well (as it always uses ints_to_bits and bits_to_ints), so this PR does not change that behaviour. It just deprecates the older functions, and always uses the QDType interfaces. And when we finalize which library to use for fixed points later, we can just update the code in QFxp without touching the rest of the codebase.

[fdmalone]

Can / should we remove the fxpmath dependency?

[mpharrigan]

I'm a little worried about this change, although I understand the frustration with Fxp. One of the primary motivations of the classical simulation protocol is to write classical programs that look like classical programs. This is super useful for a) unit testing, where one needs to inspect one side of the assertion and deem it obviously correct and b) looking at what a bloq does.

Let me think some more about this

[mpharrigan]

Can you describe the motivations, consequences, and potential alternatives for this change?

[anurudhp]

Motivation

Currently, the library mostly uses ints_to_bits and bits_to_ints from classical_sim.py to convert classical values (in Split, Join, Partition etc.), and QDType.from_bits and .to_bits to generate values in the classical simulator. This behaviour is divergent for QFxp as the former uses a fixed-width-int representation and the latter uses fxpmath.Fxp.

All existing bloqs assume the input values are integers for QFxp registers, e.g. Phase Gradient:

Qualtran/qualtran/bloqs/rotations/phase_gradient.py

Lines 312 to 325 in 15cb661

	def on_classical_vals(self, **kwargs) -> Dict[str, 'ClassicalValT']:
	x, phase_grad = kwargs['x'], kwargs['phase_grad']
	if self.controlled_by is not None:
	ctrl = kwargs['ctrl']
	if ctrl == self.controlled_by:
	phase_grad_out = (phase_grad + self.sign * self.scaled_val(x)) % (
	2**self.phase_bitsize
	)
	else:
	phase_grad_out = phase_grad
	return {'ctrl': ctrl, 'x': x, 'phase_grad': phase_grad_out}
	phase_grad_out = (phase_grad + self.sign * self.scaled_val(x)) % (2**self.phase_bitsize)
	return {'x': x, 'phase_grad': phase_grad_out}

This PR

• Consolidates the conversions, removing the classical sim functions, and always using QDType.from_bits and .to_bits.
• Use fixed-witdth-int representations always as fxpmath is unstable at the moment.

Consequences

• End users must use QFxp.to_fixed_width_int(actual_floating_point_value) to pass the correct input values to bloq.call_classically.

Potential Alternatives

• We would need a stable fixed-point library, or write our own light-weight wrappers around floats.
• We also need to explicitly update classical calls for bloqs using QFxp (like Add etc) to work with QFxp. Right now they work only for ints, and coincidentally the behaviour of the integer representations for Fxp matches this.

[mpharrigan]

Thanks for the write-up!

All existing bloqs assume the input values are integers for QFxp registers, e.g. Phase Gradient:

If this is true, then this removes some of my concern about the current PR as a near-term solution to the problem(s).

Can you add some text to the classical sim docs and classical sim docstrings that explains the behavior? Do we want to say what the integers are? The to_fixed_width_int says "re-interpret the bits as an int" but how are the bits laid out in Fxp?

A more robust approach could be to create a lightweight wrapper around ints that at least makes the user call the classical functions correctly rather than having to convert to int and back

class FxpVal:
  _as_int: int
  total_bits: int
  frac_bits: int

  def __add__(self, other):
    return self._as_int + other._as_int

  # etc

[mpharrigan]

preliminary lgtm, but some documentation suggestions

[mpharrigan]

There's some docstrings and comments hiding around the code. Can you add one or two short paragraphs to the class docstring for QFxp to describe the relationship to the classical simulation protocol and how we'll use these "raw ints" in the classical simulation protocol.

Comments don't show up in the docs and may be missed, and docstrings on private methods may be missed. You can also add a line to the docstrings of the public methods that says something like 'see the class docstring for details on the classical simulation format'

[anurudhp]

Updated the class docstring

[mpharrigan]

might be worth moving the bulk of this discussion to the class docstring and then just saying "the dtype in accordance with the format described in the class docstring"

[anurudhp]

Moved to class docstring

[mpharrigan]

Refer to a description of the format. Do we want users to know what the classical format is? Yes, probably.

[mpharrigan]

why are we using the word "template" here? because it doesn't actually have a value associated with it?

"""Prepare an empty `fxpmath.Fxp` value container for experimental fixed point support.

This constructs a `Fxp` object with no value. To assign the returned object a fixed point value, 
use ... [idk, exaplain how to use it].

Fxp support is experimental and doesn't hook into the classical simulator protocol etc etc

This corresponds to the Fxp constructor arguments:
 - op sizing, etc etc
"""

[mpharrigan]

I'd also make this a method instead of a property

[anurudhp]

I used the term from the fxpmath readme: https://github.com/francof2a/fxpmath#:~:text=It%20is%20a%20good%20idea%20create%20Fxp%20objects%20like%20template%3A

This is used to type-cast Fxp values, instead of being an empty container, hence a property.

Example usage: some_fxp_value.like(QFxp(...).fxp_dtype_template)

[anurudhp]

I've updated the docstring to explain how to use this function

[mpharrigan]

you're never going to want to plumb through any of the options?

[anurudhp]

@mpharrigan ptal! I've updated the docstrings, and added a brief explanation in the classical_sim.ipynb notebook about how to use QFxp correctly.

[mpharrigan]

lgtm after docstring formatting fixes. you can use the scripts in dev_tools to preview how the api reference docs get rendered