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

eppy converts numbers in scientific notation to plain decimal notation, which might be out of range when processed by E+ #289

Closed
wannizhang opened this issue Jun 12, 2020 · 9 comments
Assignees
Labels

Comments

@wannizhang
Copy link

It seems that when parsing numbers, eppy will convert those scientific notations (such as 1e+10) to plain decimal notations. I got a idf file with an object like this:

  ScheduleTypeLimits,
    AnyValue,                !- Name
    -1e+018,                 !- Lower Limit Value
    1e+019,                  !- Upper Limit Value
    Continuous;              !- Numeric Type

After parsed by eppy, it becomes

ScheduleTypeLimits,
    AnyValue,                 !- Name
    -1000000000000000000,     !- Lower Limit Value
    10000000000000000000,     !- Upper Limit Value
    Continuous;               !- Numeric Type

Then when it's simulated with E+, an error ** Severe ** stoll: out of range will occur, because the numbers have too many digits.
Not sure if this is a problem easy to fix in the source code. Otherwise I can always check the idf files to remove those extreme large numbers.
Thanks in advance!

@santoshphilip santoshphilip self-assigned this Jun 12, 2020
@santoshphilip
Copy link
Owner

I see this as a critical bug.
It has to be fixed.

I need a couple of days to get to this.
And I might need some more input from you.

Can you tell me which versions of E+ this happened in.
I don't think the E+ version number is significant, But it is worth looking into, to see if anything in the IDD file changed

@wannizhang
Copy link
Author

Thank you! I'm using E+ 9.2. FYI, it happens both on Windows and Mac.

@santoshphilip
Copy link
Owner

@wannizhang , Can you paste the relevant parts of the error message here
Is it possible to figure out how many digits is E+ capable of reading looking at the error message ?

@santoshphilip
Copy link
Owner

re-reading your first comment, It is clear that the limitation is E+, not in eppy.
to be practical, eppy should be coded with a workaround for that limitation.

@wannizhang
Copy link
Author

@santoshphilip Yes, I think it's a limit of E+. The .err file of E+ doesn't show the specific limit, but I tried that it allows 20 digits at most (1e19 / -1e18).
Since using scientific notations won't be out of range, I wonder if eppy can keep the scientific notations in the original file when parsing the idf.

@santoshphilip
Copy link
Owner

  • I can keep scientific notation
  • I don't want it to be scientific notation everywhere. Makes it hard to read.
  • If it is greater than x digits, I'll let it write in scientific notation.

Can you test less number of digits. Try 19, 18, 17, 16, 15. Find out where E+ stops failing. If you do that, I can update the code with some level of confidence that we are doing the right thing.

To clarify what is happening internally:
Eppy is seeing it as a floating point number or as an integer. Once eppy reads the file, it has no format (scientific notation is a printing format). it is just a number. It becomes a scientific notation or some other format, only when it saved to the disk or printed to screen.

@wannizhang
Copy link
Author

@santoshphilip Yeah I've tried different numbers of digits and it starts to fail at 20 digits (19 works).
Sure. It makes sense to only keep the scientific notation when the number is out of range.
Thanks!

santoshphilip added a commit that referenced this issue Jun 13, 2020
Problem: E+ is unable to read numbers that are wider than 19 digits
Solution: format these numbers in scientific notation
@santoshphilip
Copy link
Owner

I fixed this in branch i289_scientific
I put the cutoff at 18 digits (put a little safety margin there)

@wannizhang , can you do an E+ run and test
If it looks good I'll merge into develop branch

@santoshphilip
Copy link
Owner

merged into develop

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

No branches or pull requests

2 participants