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

Interval <-> char* conversion #190

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Interval <-> char* conversion #190

wants to merge 6 commits into from

Conversation

SimonRohou
Copy link
Member

Add a function to convert an Interval object to a C string (char*) and its reciprocal feature:

  • const char* intv2str(const Interval& intv, int precision)
  • const Interval str2intv(const char* str)

This comes with unit tests.
Will be used for robotics purposes but should be implemented inside ibex-lib.

@SimonRohou
Copy link
Member Author

(now, build and tests OK on this branch)

@SimonRohou
Copy link
Member Author

Jordan has pointed out that the intv2str function may output different strings according the low level interval library used (Filib, Gaol, etc.), thus implying a non-consistency between the two reciprocal functions. This is corrected in 29d7e9f.

@gchabert
Copy link
Contributor

Hi Simon,
I have two remarks.

  • The str2intv function is nice and useful. I would actually even turn it into a constructor of the Interval class with char* argument:
    Interval::Interval(char*) {...}

  • I'm more circumspect about the intv2str function. As a user, I would expect the following instruction for a given interval x:

    cout << setprecision(precision) << x

    to produce the same thing. Why it is not the case? If not, maybe we should reimplementostream operator<<(const Interval&) instead of creating a new function (the precision is a field of the output stream)
    Gilles

@SimonRohou
Copy link
Member Author

Hi Gilles,
I fully agree.

I updated the code, removing the new functions from ibex_String.* files.
Now the updates take place in ibex_Interval.*:

Interval(const std::string& s);
std::ostream& operator<<(std::ostream& os, const Interval& x);
std::string str(int precision = 6) const;

The str() method is a shortcut to the << operator (a direct access to a string object).
The new constructor takes a std::string object as parameter, instead of a const char*. The reason is that the compiler does not make the difference between int and const char* when dealing with 0. An ambiguity can appear, which is not the case with a std::string parameter.

So now, IBEX has its own convention for interval string conversions. I used the Filib convention.

Other improvements can be provided before the merge. You tell me.

Simon

@gchabert
Copy link
Contributor

That is perfect like this, thanks for following my suggestion; I am going to merge the pull request.
I still have a final hesitation about the str() function.
What do you think about creating a cast operator

Interval::operator std::string&()

This would allow simple usage like

string s = "my interval is" + x;

Gilles

@SimonRohou
Copy link
Member Author

Ok for the cast, but we will lose the precision parameter. No big deal since we can still use the ostream cast with setprecision.
I added the following:

inline std::string operator+(const std::string& s, const Interval& x);
inline std::string operator+(const Interval& x, const std::string& s);

In order to allow:

string s = "my interval is" + x;

Otherwise, a cast (string)x is needed.

Simon

@gchabert
Copy link
Contributor

Yes, you are right. Now I realize that it was maybe not a good suggestion... :-/
especially if an explicit cast is anyway required to concatenate strings (I don't think that the Interval class is the right place for concatenating strings)
I let you decide.
Gilles

@SimonRohou
Copy link
Member Author

Hi Gilles,
To my mind, an automatic cast is interesting, I would keep it as it is now (an explicit cast is not required in c4f23dc). The setprecision feature is less frequently used and still available through an ostream object.
But you have the long-term vision about Ibex so... as you want!
Speaking about casts, I let you know that I developed serialization methods for a tube library, including functions to write an Interval object in binary, and its reciprocal feature. If you think it might be a good idea to put it in Ibex, please let me know and I will open a dedicated branch/issue.
Simon

@gchabert
Copy link
Contributor

Dear Simon,

OK... so I guess that the string cast operator will depend now on the default precision of stringstream objects. It makes sense.

I have to say that building interval from strings is actually more complicated because of the decimal representation of floating point numbers. For example, if one writes "[0.1,0.1]", since 0.1 is actually not binary representable, using atof(0.1) will result in an approximate value of 0.1 that is either slightly smaller or greater than 0.1. In any case, the resulting interval will lose the value 0.1. So the conversion is unsafe. I remember that the string representation of intervals has been discussed in the IEEE1788 standard. Maybe you should have a look.

We can live with an unsafe conversion for the moment. But, at least, we have to write a big "warning" in the documentation.

Other remark, I think you did not take into account intervals with infinite bounds like "[3,oo]". The symbol "oo" (with potentially a sign +/-) is valid in Minibex so it should be as well in C++.
Gilles

@gchabert
Copy link
Contributor

gchabert commented Dec 7, 2016

Note: the same problem goes with minibex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants