physical unit checks

[john borland]

I took a quick look at https://github.com/unl-nimbus-lab/phriky-units

Talking to one of the developers through email he told me the project had been super seeded by https://unl-nimbus-lab.github.io/phys/

While it had more added checks he also gave me a list of issues. Which I have included below.

Phys is intended to detect a superset of the physical inconsistencies that Phriky detects.   Phys uses variable/identifier names to help infer physical units.

Both have key limitations:
* they only work with code that uses ROS messages (https://www.ros.org/). 
* Phys assumes English (for now, sorry)
* they use cppcheck as a preprocessor only, using the --dump option to generate an AST and symbol table.

I was going to take a look at the code base to see more of what they were doing, but I felt it was probably a good time to start a physical units check thread.

[Daniel Marjamäki]

I think that it only works for ROS is a major limitation that could be worked around. If they have hardcoded patterns then those should be changed into customizable patterns. I would like that if a configuration file is provided then anybody could use their tool.

With a more generic approach I would be very interested to distribute their tool together with
Cppcheck. If that is interesting for them I do not know.

[john borland]

Yeah I really hope interest align between the two projects. I know ROS has other bindings than just c++. I really like the idea of cppcheck having some way to do physical unit checks. As that check would never come from a compiler. Looking at the phys project there is a credit section where they call out the following tools.

CPPCheck
libDAI
NetworkX
Phriky
NLTK

I'm not sure what each thing is used for yet. Other than I was told they only use cppcheck --dump as a processing step. I will keep learning more about it and I will point the developer I was emailing with to this forum post.

[john borland]

It seems that NLTK is used to determine if parts of a variable name is a noun. In the Data directory there were files that matched names with units that gets pulled in as training data. I also saw it use https://en.m.wikipedia.org/wiki/Jaro%E2%80%93Winkler_distance  and  https://en.m.wikipedia.org/wiki/Levenshtein_distance

It looks like the ROS specific parts are in a file called symbol_helper.py

To me it looks like it could still use a way to anoint variables directly in the source with units. I was thinking maybe in the same way a macro is used for the limits. Any ideas?

[Daniel Marjamäki]

I was thinking maybe in the same way a macro is used for the limits. Any ideas?

one idea:

#ifndef __cppcheck__
#define __cppcheck_unit__(UNIT)
#define __cppcheck_convert__(FROM_UNIT,TO_UNIT,FROM_EXPR,TO_EXPR) TO_EXPR
#endif

typedef int __cppcheck_unit__("seconds") seconds_t;
typedef int __cppcheck_unit__("hours") hours_t;

int bad() {
    seconds_t t1 = 100;
    hours_t t2 = 10;
    return t1 + t2;  // seconds+hours!
}

#define HOURS_TO_SECONDS(T) (__cppcheck_convert__("hours", "seconds", T, (T)*3600))

int ok() {
    seconds_t t1 = 100;
    hours_t t2 = 10;
    return t1 + HOURS_TO_SECONDS(t2);  // seconds+seconds!
}

[Daniel Marjamäki]

personally.. I do not think it would be possible to annotate the projects I work on. So for me it would be necessary to have annotation free checking.

If I could say in the GUI that all variables with the name "t1" has unit "seconds" and that all variables with the name "t2" has unit "hours".. the bug in "bad" could be detected.

[john borland]

My son was born at the end of December so I'm doing more resting and thinking at this point. One thing I was wondering is do the limits from the macro show up in the ast dump?

So I have an issue were my source is already annotated with units. https://nasa.github.io/trick/documentation/building_a_simulation/Model-Source-Code#the-measurement-units-specification

I think it's something I need to play around with more. I used to have access to the international space station training simulator. So I was talking to one of the current developers to see if I could get cppcheck/phys run on it. And test out how well it works on that giant sim matches up because it is thousands of variables written by many diffrent groups that units are already marked for because it uses trick under the hood.

The goal for me is to try and figure out the most practical and non intrusive way for someone to explicitly say what the units are. I want to see how much that helps phys out.

[Daniel Marjamäki]

My son was born at the end of December so I'm doing more resting and thinking at this point.

Congratulations!

One thing I was wondering is do the limits from the macro show up in the ast dump?

It really depends. If the macro replacement list is empty then obviously it won't.

Could you create a short example code so we can look at that?

So I have an issue were my source is already annotated with units.

That trick_unit specification looks similar to my__cppcheck_unit__ suggestion. it seems to me we could probably reuse those annotations.

I used to have access to the international space station training simulator.

cool!

[john borland]

Thanks for the congratulations!

Here is a link to a cannonball sim header file they use for trick tutorials.
https://github.com/nasa/trick/blob/master/trick_sims/SIM_Ball%2B%2B_L1/models/BallState.hh

Most of the comments are just parsed using doxygen into reports. The units are using UDUINTS which does allow trick to make plots and displays on the fly in different units ( radians are fine but degrees are easier for my brain to think about) Because UDUNTS can give you the conversation between to types or tell you it's not possible. I thought that feature might be useful for cppcheck/phys to use to check unit conversation factors.

Looking more into phys I have figured out a few things.
Here is the list of errors it can find from phys/src/unit_error_types.py

`
VARIABLE_MULTIPLE_UNITS = 0

COMPARISON_INCOMPATIBLE_UNITS = 1
VARIABLE_BECOME_UNITLESS = 2
FUNCTION_CALLED_WITH_DIFFERENT_UNIT_ARGUMENTS = 3
VARIABLE_WITH_UNUSUAL_UNITS = 4
ADDITION_OF_INCOMPATIBLE_UNITS = 5
LOGICAL_OPERATOR_USED_ON_UNITS = 6
UNIT_SMELL = 7
`

Also it actually runs the constraint solver 4 times. Each time it runs it takes all the new units it has matched and runs them as new contains to the solver. To me this is intresting because in the traditional static analysis this is fine but thinking in terms of bug hunting you might want to examine the outputs between each run of constraint solver to really get a maximum effort out of it.

I can say from what I'm seeing phys has good guts to it from what I've seen, but I think I can help expose some of it's knobs and levers to help make it more versatile.

When I was first thinking about bug hunting it really stood out as a feature that really would of benefited from contracts in c++20. Because it could of parsed the constraints out of the contracts. You wouldn't even how had to come up with a cppcheck specific syntax you could of just made it match c++20. So when I found out contracts were no longer in c++20 I was like crap now what. So I can see reasons for not wanting to develop too much cppcheck specific code to anoint source files. Just incase contracts make it into c++23. Right now I still think looking into physical unit checks would bring a very useful feature to cppcheck that isn't done by other static analysis tools or compiler checks/warnings. To me right now I'm looking at this as a very interesting opportunity in something that I'm find very interesting. I am trying to keep some of the bigger picture in mind. Like should all physical unit checks be in phys? Right now that's how I'm leaning. But would anyone ever want to write a library cfg with physical units in it? How would I get that info to Phys? I still have a lot more to learn and look into so I will keep plugging away at it.

Also I know a lot of developers at NASA that use cppcheck, but most of us don't use the GUI. It really would be nice to have command line ways to add in constraints.