openrocket / openrocket

Model-rocketry aerodynamics and trajectory simulation software
https://openrocket.info/
Other
1.27k stars 402 forks source link

Unit rounding creating discontinuity issues. #999

Closed hcraigmiller closed 1 year ago

hcraigmiller commented 3 years ago

See prior discussion thread, https://github.com/openrocket/openrocket/issues/871#issuecomment-899110349

SiboVG commented 3 years ago

1036 is an issue that also arose from the confusion of rounding off the input fields to single decimal numbers.

neilweinstock commented 2 years ago

I'm a little unclear what has been fixed and what still needs addressing, regarding decimal precision. If there are still problems that lead to discontinuity errors, I'm thinking this should maybe be a release blocker, because mysterious discontinuity warnings are an extremely common source of confusion and questions on TRF, and if they're the fault of the program rather than the users then it really should be fixed. But it depends on how messy (or not) the solution would be.

I'll await further clarity before labeling.

SiboVG commented 2 years ago

Just want to put out a random thought before I forget it: maybe it's an idea to be able to right-click a value entry box and set the number of displayed decimals?

Demo: ezgif com-gif-maker

This could help users who want to display higher precision numbers than the default ones (like in #1036).

hcraigmiller commented 2 years ago

This seems more convenient than on the preferences tab, so long as changing the decimal place for one length changes the decimal place for all lengths, diameter for all diameters, etc.

hcraigmiller commented 2 years ago

Additional information about this bug is derived from a post on the FaceBook OpenRocket group. The user created a design that exibited a discontinuity error with no visible explanation as to why; all outer diameters appeared to be the same.

Deconstruction (stripped file) revealed that the discontinuity related to a disparity between the outer diameter of the Payload Bay (an Estes component) and the Sustainer Airframe (a custom component). Changing the characteristics of the Sustainer Airframe, (1) checking the automatic box removes the discontinuity warning, (2) re-entering the outer diameter does NOT remover the warning, (3) manually changing the outer diameter (entering the change), then re-entering the original outer diamter removes the discontinuity warning. Looking at the saved file with the discontinuity warning, the Payload Bay (Estes tube) has a radius of 0.0093472 and the Sustainer Airframe (custom component) has a radius of 0.00935.

Stegeman, Vaughn.Canopus Archangel Sounding Rocket A03.ork.txt (Original .ork file)

Stegeman, Vaughn.Canopus Archangel Sounding Rocket A03.Stripped.ork.txt (Stripped .ork file)

neilweinstock commented 2 years ago

Ugly.

But also reinforces (for the umpteenth time) that every single warning and error should specify exactly where it comes from. Just saying "Discontinuity error" without identifying the two components and the offending values is super dumb.

artao5 commented 2 years ago

Additional information about this bug is derived from a post on the FaceBook OpenRocket group. The user created a design that exibited a discontinuity error with no visible explanation as to why; all outer diameters appeared to be the same.

Deconstruction (stripped file) revealed that the discontinuity related to a disparity between the outer diameter of the Payload Bay (an Estes component) and the Sustainer Airframe (a custom component). Changing the characteristics of the Sustainer Airframe, (1) checking the automatic box removes the discontinuity warning, (2) re-entering the outer diameter does NOT remover the warning, (3) manually changing the outer diameter (entering the change), then re-entering the original outer diamter removes the discontinuity warning. Looking at the saved file with the discontinuity warning, the Payload Bay (Estes tube) has a radius of 0.0093472 and the Sustainer Airframe (custom component) has a radius of 0.00935.

Stegeman, Vaughn.Canopus Archangel Sounding Rocket A03.ork.txt (Original .ork file)

Stegeman, Vaughn.Canopus Archangel Sounding Rocket A03.Stripped.ork.txt (Stripped .ork file)

Thanks for posting this. I am the designer of the Canopus Archangel. :) EDIT: BINGO!! Thank you. I changed the construction material of the offending tube to an Estes BT-20 457mm and no more body discontinuity. I have a bulk pack of tubes from Apogee Components. I like them over Estes because they aren't glassine covered and thus require less pre-prep work during construction and finishing. Also, in early design I'm not concerned about material and just "go wild" manually adjusting parameters that get me something I like. Not to mention that I've got a whole bunch of general tubes NOT made for model rocketry. Another reason for the scale, to calculate their density. Not just for rocket design, but for fun! :D I JUST ordered a nice vernier calipers and digital scale accurate to 0.001. I'll see what I can do about calculating up a set of data for them.

hcraigmiller commented 1 year ago

In reviewing numerous designs posted on several threads, I found that the most common discontinuity error could not be seen by looking at the component diameter values. There are likely two possible causes for this, (1) a portion of the airframe was scaled and then a standard airframe component added (for discontinuity purposes, the scaled diameter is not rounded up or down to the number of decimal places in the displayed value) and (2) manually entered values are beyond the precision of the decimal places in the displayed value, and are slightly different (using inches for instance, 1.1875 and 1.1876 both display as 1.188, but trigger a discontinuity error).

The fix would seem to be for the discontinuity flag to trigger the warning when the precision of the value displayed (applied to the component diameters, as if rounding the component diameters to that level of precision) is not the same; the actual, more precise value would be retained for all other purposes. Using the example above, for a three decimal places of precision, because both 1.1875 and 1.1876 round to 1.188 so no discontinuity error would be displayed.

So long as this is not a time-consuming fix, I would suggest that it be a release blocker.

neilweinstock commented 1 year ago

I'm not convinced that the 4th decimal place (5th significant digit in this case) is worth preserving at all... but yes, at the very list, limiting the discontinuity checking to the number of displayed decimal places would be a start to eliminating the absolute worst of the warnings, where the visible dimensions are identical. I don't know off the top of my head if there are any particular pitfalls in doing this.

JoePfeiffer commented 1 year ago

I'm not sure how big a discontinuity is worth telling the user about -- but if it's worth displaying a warning, it's worth displaying as different in the text fields. My intuition is that a difference in the fifth significant digit doesn't matter (my caliper doesn't display that many digits, and I sure wouldn't be able to measure accurately at that level if it could), but the one thing I'm sure of is that if we display the diameters as equal, the software should regard them as equal and not put up the warning.

neilweinstock commented 1 year ago

the one thing I'm sure of is that if we display the diameters as equal, the software should regard them as equal and not put up the warning.

It's impossible to argue with that. The body discontinuity warning could use more work in the future but implementing this one common-sense behavior would seem to be a good small step for this release, if it's not too much of a mess to implement.