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

limits should be stored in the connection table #44

Open
philipstarkey opened this issue Jan 23, 2018 · 5 comments
Open

limits should be stored in the connection table #44

philipstarkey opened this issue Jan 23, 2018 · 5 comments
Labels
bug Something isn't working minor

Comments

@philipstarkey
Copy link
Member

Original report (archived issue) by Philip Starkey (Bitbucket: pstarkey, GitHub: philipstarkey).


Currently limits only protect you from breaking something when running a shot, but are not respected by BLACS since they aren't saved anywhere.

Device limits (really only relevant to analog quantities and derivatives) should be stored somewhere so that BLACS can access them and use them to protect devices. I'm not sure that connection table properties is really the best idea (should probably be device properties) but since we don't have device properties working for channels (they only work for devices) then the connection table is really the only place left to us.

We should probably also document them (at least show an example in example.py) as probably most people reading this won't even have known limits exist!

@philipstarkey
Copy link
Member Author

Original comment by Jan Werkmann (Bitbucket: PhyNerd, GitHub: PhyNerd).


I'm in favor of this change but I see one problem with storing limits in connection table properties. The limits of a experiment file need to be the exact same as those of the connection table or connection table comparison fails.
It would make more sense if any limits that are within the limits of the connection table would be allowed.
So experiment limits (2, 3) with connection table limits of (1, 5) should pass comparison but experiment limits of (2, 6) with connection table limits of (1, 5) should fail.

This could be done by checking them separately in Connection.compare_to.
Is this the way to go or does anyone have a better idea? I'd be happy to make the 2 pull requests if there should be no other suggestions on how to get around this.

@philipstarkey
Copy link
Member Author

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: philipstarkey).


Unfortunately I don't have a better idea. I don't particularly like having to treat "limits" as a special key of the connection table properties dictionary, but don't see any other option other than not allowing you to have limits that differ from the connection table.

I'm not sure...maybe you shouldn't be able to set limits that are different to the main connection table? What are the scenarios where you might want different limits set?

@philipstarkey
Copy link
Member Author

Original comment by Jan Werkmann (Bitbucket: PhyNerd, GitHub: PhyNerd).


Don't have any example handy, but I would feel better with allowing subranges. This prevents the user from recompiling the connection table in some cases and that always a good thing.
I'd consider the connection table limits to be the values to not be exceeded to save connected hardware failure or damage.
In a experiment script however one could try to not exceed a sublimit (for some obscure reason) and the compiler could help do that.

@philipstarkey
Copy link
Member Author

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: philipstarkey).


Another idea I have is that we could encode the method of comparison inside the key of the JSON dictionary. This is similar to the way you define SQL clauses using Pythonic syntax in Django. For example, instead of saving it as 'limits': [0, 10] we could save it as 'limits__allow_interval_subset': [0,10] to indicate the values that are a subset are allowed during connection table comparisons.

Not sure if I like this a 100%, but could be preferable to hardcoding a special case for limits since it's a) more general (we update the connection table comparison to handle __allow_interval_subset for any parameter in the connection table properties) and b) contains information about the need to do a special comparison in the connection table itself (so it is self-documenting, which is a key principle of the labscript suite).

Note: I realise __allow_interval_subset is an awful suffix, it was just a descriptive example and not necessarily what I would want it to be!

@philipstarkey
Copy link
Member Author

Original comment by Jan Werkmann (Bitbucket: PhyNerd, GitHub: PhyNerd).


While that sounds more elegant it is equivalent in my opinion. I don't think there is any other case where we store a 2-tuple in the connection table properties that would need this subset feature. Also limits allowing subsets doesn't really need documenting as that is expected behavior.
So I'd opt for making limits special and checking it separately in compare to.

@philipstarkey philipstarkey added minor bug Something isn't working labels Apr 5, 2020
Loki27182 pushed a commit to Loki27182/labscript that referenced this issue Oct 9, 2023
Do not assume `__file__` is not None if it exists.

Approved-by: Philip Starkey <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working minor
Projects
None yet
Development

No branches or pull requests

1 participant