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

Constructing 'constrained' objects with empty constraints #1524

Open
JacquesCarette opened this issue Jun 9, 2019 · 13 comments
Open

Constructing 'constrained' objects with empty constraints #1524

JacquesCarette opened this issue Jun 9, 2019 · 13 comments
Labels

Comments

@JacquesCarette
Copy link
Owner

Currently it is possible to build something (say via constrained') that is "constrained" but in fact has an empty list of constraints (there are examples in Game Physics). This is a double bug:

  1. in the infrastructure, which allows this
  2. in usage, which uses this.

@danscime can you please augment this Issue with a list of all of the uses of this buggy feature? We need to know the scale of the problem before we 'just fix it'. Then this can be assigned to the appropriate people.

@danscime
Copy link
Collaborator

Searched project for usages of:
uq, uqc, uqcND, cvc, constrained', cuc, cuc', cuc'', constrainedNRV', cnstrw, cnstrw', uvc

Found empty list usages:

Using constrained':

GamePhysics Unitals:
  gravAccelCons
  posCons
  veloCons
  orientCons
  angVeloCons
  forceCons
  torqueCons

Using cuc':

SSP Unitals:
  coords

Using uqc:

SSP Unitals:
  slopeDist
  slopeHght
  waterDist
  waterHght
  xMaxExtSlip
  xMaxEtrSlip
  xMinExtSlip
  xMinEtrSlip
  yMaxSlip
  yMinSlip

@JacquesCarette
Copy link
Owner Author

Each one of these look like they need a 'domain expert' to look at it, to decide whether there should be a constraint [and add it], or there shouldn't be, and a different constructor should be used. @smiths can you do that task assignment please? Then we should make it illegal to use the constructors that expect a constraint to get none.

@smiths
Copy link
Collaborator

smiths commented Jul 25, 2019

@JacquesCarette, are we just taking about physical constraints? That is constraints for things that physically cannot happen? We distinguish these from software constraints. The software constraints are not imposed by the "universe," but by the context of the specific problem. For instance, x and y coordinates can take on any value between -inf and inf. They are not constrained.
However, for a specific problem, there may be bounds. We don't really know this until we look at the specific problem. This happens for the solar water heating tank for its dimensions. Physics requires that the dimensions be greater than zero, but engineering common sense puts other limits, like Lmin <= L <= Lmax.

@oluowoj
Copy link
Collaborator

oluowoj commented Jul 25, 2019

Searched project for usages of:
uq, uqc, uqcND, cvc, constrained', cuc, cuc', cuc'', constrainedNRV', cnstrw, cnstrw', uvc
Found empty list usages:
Using constrained':
GamePhysics Unitals:
gravAccelCons
posCons
veloCons
orientCons
angVeloCons
forceCons
torqueCons

Using cuc':
SSP Unitals:
coords

Using uqc:
SSP Unitals:
slopeDist
slopeHght
waterDist
waterHght
xMaxExtSlip
xMaxEtrSlip
xMinExtSlip
xMinEtrSlip
yMaxSlip
yMinSlip

I was just looking at the physics example yesterday and tried to see if there is a constructor that can be used specifically for empty constraints instead of using constrained' for those ones, I have not found, let me know if you find one.

@JacquesCarette
Copy link
Owner Author

@smiths you are over-complicating things: my issue is much simpler. We are using a constructor for "constrained" things when in fact we give no constraints. This feels like a bug.

@oluowoj if you're trying to build something with empty constraints, then you ought to be building something "lower down" in the hierarchy of quantities. There's a dependency graph of the classes somewhere, that should be your guide.

@smiths
Copy link
Collaborator

smiths commented Jul 31, 2019

@JacquesCarette what are you looking for from me? Yes, we shouldn't constrain things that do not have constraints.

@JacquesCarette
Copy link
Owner Author

@smiths : I want someone knowledgeable to review each use of an empty constraint to figure out whether that is correct (and thus a different constructor should be used), or there ought to be a constraint but it's not there. I would like you to do the assignment of that review to the 'right' people in the team, given the domain of each use, as given in @danscime 's list.

@smiths
Copy link
Collaborator

smiths commented Aug 1, 2019

@JacquesCarette based on your response, I'll assume that you are only interested in the physical constraints. The software constraints would be problem specific, but physics constraints will always apply. I'll go through the list from @danscime and identify any missing physical constraints. My guess is that most of the items in his list do not have physical constraints, except maybe for some that should be constrained to be positive. @danscime, to save me some time, can you repeat your list again, but this time include the definition of each object?

@danscime
Copy link
Collaborator

danscime commented Aug 1, 2019

I think some more have been added and removed since I made the list, so I will include updated ones here.

Using constrained':

GamePhysics Unitals:
gravAccelCons - Gravitational constant (aside: not acceleration, should maybe be renamed)
posCons - General Position (given a typical value, but not sure how it is used.)
veloCons - General Velocity (given a typical value, but not sure how it is used.)
angVeloCons - Angular velocity
forceCons- General Force (given a typical value, but not sure how it is used.)
torqueCons - Torque
posOutCons - General Position output
veloOutCons - General Veolcity output
angVeloOutCons - General Angular Velocity output

Using cuc':

SSP Unitals:
coords - General x and y coordinate pair (not sure how it is used.)

Using uqc:

SSP Unitals:
slopeDist - x-coordinates of points on the soil slope
slopeHght - y-coordinates of points on the soil slope
waterDist - x-positions of the water table
waterHght - heights of the water table
xMaxExtSlip - maximum potential x-coordinate for the exit point of a slip surface
xMaxEtrSlip - maximum potential x-coordinate for the entry point of a slip surface
xMinExtSlip - minimum potential x-coordinate for the exit point of a slip surface
xMinEtrSlip - minimum potential x-coordinate for the entry point of a slip surface
yMaxSlip - maximum potential y-coordinate of a point on a slip surface
yMinSlip - minimum potential y-coordinate of a point on a slip surface

@bmaclach
Copy link
Collaborator

bmaclach commented Aug 1, 2019

I can speak to the ones from SSP since I worked with those. All of the ones under the "Using uqc" heading should indeed have constraints, but the constraints were too complicated to capture given what we currently have in Drasil. The constraints were instead written in words in the "Notes" section of IM:crtSlipId.

coords is used a couple times throughout the document to generically refer to coordinates without having to specify if they are slope coordinates, water table coordinates, slip surface coordinates, or critical slip surface coordinates. It's probably a hack and should be replaced with the specific types of coordinates wherever used.

@JacquesCarette
Copy link
Owner Author

@bmaclach we should make issues for each of those 'too complicated to capture'. They should be eventually fixed. But in the meantime, we should fix things to use the right constructor: I really want to upgrade the 'constrained' constructors to throw an error when there are none given!

@bmaclach
Copy link
Collaborator

bmaclach commented Aug 2, 2019

Issues #1795, #1796, #1797, #1798, #1799, #1800 created. #453 for monotonicity constraints already existed.

@smiths
Copy link
Collaborator

smiths commented Aug 6, 2019

Thank you @danscime. This information is very helpful. I'll edit in my thoughts below.

I think some more have been added and removed since I made the list, so I will include updated ones here.

Using constrained':

GamePhysics Unitals:
gravAccelCons - Gravitational constant (aside: not acceleration, should maybe be renamed)

Yes, this should be renamed. (@danscime, can you please create an issue for this.) If I remember correctly, this is the gravitational constant from Newton. It definitely does not have a constraint. It is a constant. I guess it has a constraint that it can only take on one value, but if we have the notion of constants, then we wouldn't express this with a constraint.

posCons - General Position (given a typical value, but not sure how it is used.)

I cannot think of any constraints for this. Any value (real number) should be valid, from a physics
point of view.

veloCons - General Velocity (given a typical value, but not sure how it is used.)

I cannot think of any constraints for this. Any value (real number) should be valid, from a physics point of view.

angVeloCons - Angular velocity

I cannot think of any constraints for this. Any value (real number) should be valid, from a physics point of view.

forceCons- General Force (given a typical value, but not sure how it is used.)

I cannot think of any constraints for this. Any value (real number) should be valid, from a physics point of view.

torqueCons - Torque

I cannot think of any constraints for this. Any value (real number) should be valid, from a physics point of view.

posOutCons - General Position output

I cannot think of any constraints for this. Any value (real number) should be valid, from a physics point of view.

veloOutCons - General Velocity output

I cannot think of any constraints for this. Any value (real number) should be valid, from a physics point of view.

angVeloOutCons - General Angular Velocity output

I cannot think of any constraints for this. Any value (real number) should be valid, from a physics point of view.

Using cuc':

SSP Unitals:
coords - General x and y coordinate pair (not sure how it is used.)

@bmaclach is right that this should probably be changed to the specific coords that are being referenced. In the mean time, we cannot reasonably constrain this variable.

Using uqc:

SSP Unitals:
slopeDist - x-coordinates of points on the soil slope
slopeHght - y-coordinates of points on the soil slope
waterDist - x-positions of the water table
waterHght - heights of the water table
xMaxExtSlip - maximum potential x-coordinate for the exit point of a slip surface
xMaxEtrSlip - maximum potential x-coordinate for the entry point of a slip surface
xMinExtSlip - minimum potential x-coordinate for the exit point of a slip surface
xMinEtrSlip - minimum potential x-coordinate for the entry point of a slip surface
yMaxSlip - maximum potential y-coordinate of a point on a slip surface
yMinSlip - minimum potential y-coordinate of a point on a slip surface

@bmaclach has discussed these in the issues below.

@danscime danscime removed their assignment Aug 20, 2019
@JacquesCarette JacquesCarette mentioned this issue May 14, 2020
2 tasks
@balacij balacij added the bug label Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants