Ticket #9427 (inprogress)

Opened 6 years ago

Last modified 5 years ago

In InstrumentDefinitionParser: use of atoi and boost::lexical_cast consistent

Reported by: Anders Markvardsen Owned by: Federico M Pouzols
Priority: minor Milestone: Backlog
Component: Framework Keywords: Maintenance
Cc: martyn.gigg@… Blocked By: #9406
Blocking: Tester:

Description

Originally atoi and atof used to convert string to int and double.

At recently boost::lexical_cast used, which is generally recommended over using atoi and atof, although as was found it #9406 boost::lexical_cast does not strip for spaces.

This ticket is proposed to translate all atoi and atof to boost::lexical_cast.

In doing this it has to be checked that the performance of loading IDF does not increase (or only increase very little).

One way of implementing this is to add a private method getStripedAttValue() which returns a string that can be passed to boost::lexical_cast

Change History

comment:1 Changed 6 years ago by Anders Markvardsen

  • Cc martyn.gigg@… added

comment:2 Changed 6 years ago by Anders Markvardsen

  • Keywords Maintenance added

comment:3 Changed 6 years ago by Federico M Pouzols

  • Owner set to Federico M Pouzols
  • Status changed from new to assigned

comment:4 Changed 6 years ago by Federico Montesino Pouzols

  • Status changed from assigned to inprogress

Use boost::lexical_cast rather than atoi/atof, re #9427

Changeset: 5d761ec5f809413ec5810364bf206455b5a8a18c

comment:5 Changed 6 years ago by Federico M Pouzols

I tried replacing the remaining atoi/atof by nested calls to String::strip then boost::lexical_cast to make the InstrumentDefinitionParser consistent in this regard. But I had to step back as this definitely creates issues with tests. Approximation differences between the two alternatives (atoi/f and lexical_cast) make 16 tests fail: http://builds.mantidproject.org/job/develop_incremental/2755/testReport/. For those it should be carefully checked whether it is safe to just increase the error epsilon in the approximate comparisons of results (or add an epsilon, as some tests are doing an exact comparison). This issue occurs on all platforms, and seems to be caused by a difference in precision between atof and stringstream operators.

More in general, there are still quite a few atoi/atof calls in the Mantid code, though not so many, roughly 60 atoi and 115 atof. These are very spread through different components though. There are also a few tens of strtod/strtol/strtof. Maybe a lowish priority maintenance ticket could be created for this. Many issues in the tests can be expected though.

comment:6 Changed 6 years ago by Martyn Gigg

I agree this should definitely be moved to back as maintenance issue as it's too close to release time to be making low-level changes like this and this was more of a clean up than fixing a known issue.

comment:7 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 10270

Note: See TracTickets for help on using tickets.