labrad / servers

LabRAD servers
24 stars 21 forks source link

Setting Hittite frequency fails on non integers #90

Open DanielSank opened 9 years ago

DanielSank commented 9 years ago

The problem is roughly that

freq_Hz = freq['Hz']
yield self.write('SOUR:FREQ:FIX %f' %freq_Hz)

fails when freq_Hz is not a legal value accepted by the Hittite. In this case, the Hittite seems to just ignore the command.

Copying from #89:

Dan wrote:

There is a deeper issue here: servers which set hardware parameters should always read back the parameter they try to write and fail if the attempted write value does not match the read-back.

I think the change proposed here is better than what was there before because in the old way the result was that the write would silently do nothing. Now you at least get a frequency which is within 1 Hz of what you wanted. I'm all ears if folks think this sucks. If we had a LabRAD type for unit-checked integers this would be much easier to deal with, but we don't. Ideas?

Evan wrote:

I disagree. I don't think it is generally useful or possible to insist that the floating point representation match an exact digital value. If rounding a value to the nearest representation doesn't make sense, the server setting should be changed to take an integer. Any code which expects floating point values to have exact results is broken.

Consider this: the fastbias cards cannot be set to 1.0 volts. The nearest legal value is approximately 999.985 mV. Should that server really fail if you ask it to produce 1 volt?

There are basically three ways to deal with this:

1) write code whose correctness does not depend on small changes needed to coerce to a legal value 2) Eliminate floating point numbers and used only fixed point for anything that will be used to set a hardware value 3) Query the device after setting it to learn the actual value it was configured for, and base further computation off of that. Keep in mind that devices like frequency generators that use an internal decimal representation may not return something that is exactly representable as a floating point number, so this may not even fix the problem.

DanielSank commented 9 years ago

I don't think it is generally useful or possible to insist that the floating point representation match an exact digital value.

That is precisely why having a unitful integer type would be useful. This type would be able to convert to smaller units, but not bigger ones. This automatically takes care of not trying to set a device's parameter finer than it's allowed resolution.

If rounding a value to the nearest representation doesn't make sense

Rounding to the nearest representation is exactly what my change does, so now I don't understand the objection. I would say, however,

Any code which expects floating point values to have exact results is broken.

I agree. Who said otherwise?

Consider this: the fastbias cards cannot be set to 1.0 volts. The nearest legal value is approximately 999.985 mV. Should that server really fail if you ask it to produce 1 volt?

Yes.

1) write code whose correctness does not depend on small changes needed to coerce to a legal value

I can't imagine how to make that work.

2) Eliminate floating point numbers and used only fixed point for anything that will be used to set a hardware value

Fixed point unitful type, please.

3) Query the device after setting it to learn the actual value it was configured for, and base further computation off of that. Keep in mind that devices like frequency generators that use an internal decimal representation may not return something that is exactly representable as a floating point number, so this may not even fix the problem.

This would be fine but I don't trust people to do this consistently. It puts too much responsibility on the programmer.

ejeffrey commented 9 years ago

Sorry, I misread the patch. Based on your pull request message I thought you were going to cause an error if rounding changed the value, i.e., the input were not an exact integer number of Hz.

The rounding is fine.

On Sat, Apr 11, 2015 at 3:31 PM, Daniel Sank notifications@github.com wrote:

I don't think it is generally useful or possible to insist that the floating point representation match an exact digital value.

That is precisely why having a unitful integer type would be useful. This type would be able to convert to smaller units, but not bigger ones. This automatically takes care of not trying to set a device's parameter finer than it's allowed resolution.

If rounding a value to the nearest representation doesn't make sense

Rounding to the nearest representation is exactly what my change does, so now I don't understand the objection. I would say, however,

Any code which expects floating point values to have exact results is broken.

I agree. Who said otherwise?

Consider this: the fastbias cards cannot be set to 1.0 volts. The nearest legal value is approximately 999.985 mV. Should that server really fail if you ask it to produce 1 volt?

Yes.

1) write code whose correctness does not depend on small changes needed to coerce to a legal value

I can't imagine how to make that work.

2) Eliminate floating point numbers and used only fixed point for anything that will be used to set a hardware value

Fixed point unitful type, please.

3) Query the device after setting it to learn the actual value it was configured for, and base further computation off of that. Keep in mind that devices like frequency generators that use an internal decimal representation may not return something that is exactly representable as a floating point number, so this may not even fix the problem.

This would be fine but I don't trust people to do this consistently. It puts too much responsibility on the programmer.

— Reply to this email directly or view it on GitHub https://github.com/martinisgroup/servers/issues/90#issuecomment-91935856 .