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

Average values for integer output variables fail debug assert and are rounded to 2 places #9000

Closed
2 of 3 tasks
mjwitte opened this issue Aug 23, 2021 · 1 comment · Fixed by #9294
Closed
2 of 3 tasks
Assignees
Labels
Defect Includes code to repair a defect in EnergyPlus

Comments

@mjwitte
Copy link
Contributor

mjwitte commented Aug 23, 2021

Issue overview

Defect file requests an integer output variable, Site Rain Status, for timestep, hourly, and monthly. When the averaged value is reported in a debug build, it trips on this assert in strip_trailing_zeros which is called from here in WriteReportIntegerData. Looks like this needs to use a different format for the actual integer timestep values vs the real averaged value.

When the defect file is run with a release build, the April average is reported as 0.03 while the average of the timestep values is 0.033529. Seems like averages of integers should report more than two decimal places?

Details

Some additional details for this issue (if relevant):

  • Platform Win64
  • Version of EnergyPlus 9.6.0 (almost I/O Freeze)

Checklist

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

@jmarrec jmarrec self-assigned this Feb 21, 2022
@jmarrec jmarrec added the Defect Includes code to repair a defect in EnergyPlus label Feb 21, 2022
@jmarrec
Copy link
Contributor

jmarrec commented Feb 22, 2022

At v9.3.0, prior to moving away from gio to fmt:

ObjexxFCL::gio::write(NumberOut, fmtLD) << repVal;
strip_trailing_zeros(strip(NumberOut));

So it should be be strip, not trim. That part is easy. What to do with the actual formatting is much harder (I've been banging my head at it for hours, for fear of changing the output)

First datapoint, I made a custom build of objexxFCL extracted from E+ source at v9.3.0, and added a test

TEST( GioTest, Testbed )
{
    int unit1( get_unit() );
    EXPECT_TRUE( unit1 > 0 );
    std::string const name1( "GioTestBed.txt" );
    open( unit1, name1 );

    double repVal = 0.03576388888888889;

    write( unit1, "(A)" ) << "Line 1";

    const ObjexxFCL::gio::Fmt fmtLD("*");
    ObjexxFCL::gio::write(unit1, fmtLD) << repVal;

    rewind( unit1 );
    std::string line;
    read( unit1, "(A)" ) >> line;
    EXPECT_EQ( "Line 1", line );
    read( unit1, "(A)" ) >> line;
    EXPECT_EQ("  3.576388888888889E-002", line);
}

So it seemed that before 3.576388888888889E-002 was the output, versus the current output of 0.3E-01.
So that would be more akin to the output from:

double val = 0.03576388888888889;
fmt::print(":24.15E = {:24.15E}\n", val);

The big question is what should be a good default here?** 0.576388888888889E-01 might be overkilled as far as precision goes?


Way too much information: Tracking it down in ObjexxFCL::gio

Click to expand

Before the move away from gio, it was doing

static ObjexxFCL::gio::Fmt fmtLD("*");

static ObjexxFCL::gio::Fmt fmtLD("*");
void WriteReportIntegerData(..., Real64 const repValue, ...) {
[...]
Real64 repVal = repValue:
ObjexxFCL::gio::write(NumberOut, fmtLD) << repVal;
strip_trailing_zeros(strip(NumberOut));

So the selected option was:

Format.cc:

    void
    FormatLD::out( std::ostream & stream, double const v, std::string const & ter )
    {
        stream << spc( ter ) << fmt::LD( v );
    }

fmt.hh:

// List-Directed: double Specialization
inline
std::string
LD( double const v )
{
    typedef  TraitsLD< double >  Tr;
    return G( v, Tr::w, Tr::d, Tr::e, 1 );
}

TraitsLD.hh

// TraitsLD: double Specialization
template<>
struct TraitsLD< double >
{
    typedef  double  traits_type;
    typedef  std::size_t  Size;

    static Size const w = 24; // Field width
    static Size const d = 15; // Fraction width
    static Size const e = 3; // Exponent width
};

Now the actual call:

fmt.hh:

w=24, d=15, e=3, k=1

p is resolved to -1, so we end up calling return E( t, w, d, e, k );

// General
template< typename T >
inline
std::string
G( T const & t, Size w = TraitsG< T >::w, Size const d = TraitsG< T >::d, Size const e = TraitsG< T >::e, int const k = 0 )
{
    if ( w == NOSIZE ) w = TraitsG< T >::w;
    if ( std::numeric_limits< T >::is_integer ) { // Integer
        return I( t, w );
    } else { // Treat as floating point
        T const m( std::abs( t ) );
        if ( m == T( 0.0 ) ) {
            Size const n( std::min( e + 2, w ) );
            return F( t, w - n, d - 1 ) + std::string( n, ' ' );
        } else {
            int const p( static_cast< int >( std::floor( std::log10( m ) + T( 1 ) ) ) ); // Fortran 2003 rounding modes are not supported
//          int const p( static_cast< int >( std::log10( m / ( T( 1.0 ) - ( T( 0.5 ) * std::pow( T( 10.0 ), -d ) ) ) ) + T( 1.0 ) ) ); // "Compatible" rounding mode: r = 0.5
            if ( ( 0 <= p ) && ( p <= int( d ) + 2 ) ) { // Use F editing
                Size const n( std::min( e + 2, w ) );
                return F( t, w - n, d - std::min( Size( p ), d ) ) + std::string( n, ' ' );
            } else { // Use E editing
                if ( w == 0ul ) { // Choose width
                    Size const e_( TraitsG< T >::e ); // G0.dEe form not allowed in Fortran: Set exponent width based on type
                    return E( t, d + e_ + 4, d, e_, k );
                } else { // Use specified width
                    return E( t, w, d, e, k );
                }
            }
        }
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Defect Includes code to repair a defect in EnergyPlus
Projects
None yet
2 participants