Skip to content

Large int bug #238

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

Merged
merged 8 commits into from
Apr 12, 2022
Merged

Large int bug #238

merged 8 commits into from
Apr 12, 2022

Conversation

MauriceHendrix
Copy link
Contributor

@MauriceHendrix MauriceHendrix commented Apr 11, 2022

Description

Fixed an issue with printing large ints

Motivation and Context

There was an issue where in some environments (but not all) large numbers such as 8.034023767017109e+27 whch were ints would be printed as an the int 8034023767017108950029959168 and then the C++ compiler would complain as that's more than the maximum int value. This has been fixed by only printing ints as ints if they are MIN_INT < number < MAX_INT else we print them as a float (and we get the sceintific notation).

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Documentation checklist

  • I have updated all documentation in the code where necessary.
  • I have checked spelling in all (new) comments and documentation.
  • I have added a note to RELEASE.md if relevant (new feature, breaking change, or notable bug fix).
  • I have updated version & citation.txt & citation.cff version.

Testing

  • Testing is done automatically and codecov shows test coverage
  • This cannot be tested automatically

@MauriceHendrix MauriceHendrix marked this pull request as ready for review April 11, 2022 16:01
@codecov
Copy link

codecov bot commented Apr 11, 2022

Codecov Report

Merging #238 (c0daf47) into master (3aa2b20) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            master      #238    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           26        30     +4     
  Lines         1522      1775   +253     
==========================================
+ Hits          1522      1775   +253     
Impacted Files Coverage Δ
chaste_codegen/__init__.py 100.00% <100.00%> (ø)
chaste_codegen/_chaste_printer.py 100.00% <100.00%> (ø)
chaste_codegen/_command_line_script.py 100.00% <100.00%> (ø)
chaste_codegen/_labview_printer.py 100.00% <100.00%> (ø)
chaste_codegen/_linearity_check.py 100.00% <100.00%> (ø)
chaste_codegen/_lookup_tables.py 100.00% <100.00%> (ø)
chaste_codegen/_optimize.py 100.00% <100.00%> (ø)
chaste_codegen/_partial_eval.py 100.00% <100.00%> (ø)
chaste_codegen/_rdf.py 100.00% <100.00%> (ø)
chaste_codegen/backward_euler_model.py 100.00% <100.00%> (ø)
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5fe5344...c0daf47. Read the comment docs.

@MauriceHendrix
Copy link
Contributor Author

@kwabenantim I'm not expecting you to review the code, but let me know if this solves the issue, if it does I'll release this version.

@MauriceHendrix
Copy link
Contributor Author

There was an issue where in some environments (but not all) large numbers such as 8.034023767017109e+27 whch were ints would be printed as an the int 8034023767017108950029959168 and then the C++ compiler would complain as that's more than the maximum int value. This has been fixed by only printing ints as ints if they are MIN_INT < number < MAX_INT else we print them as a float (and we get the sceintific notation).

@MauriceHendrix MauriceHendrix merged commit cb5d1b9 into master Apr 12, 2022
@MauriceHendrix MauriceHendrix deleted the large_int_bug branch April 12, 2022 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant