Skip to content

some bug fixes#17

Merged
ibell merged 1 commit intoCoolProp:masterfrom
rafuck:master
Feb 19, 2017
Merged

some bug fixes#17
ibell merged 1 commit intoCoolProp:masterfrom
rafuck:master

Conversation

@rafuck
Copy link
Copy Markdown
Contributor

@rafuck rafuck commented Feb 18, 2017

  1. fix all warnings in g++ -Wall -Wextra
  2. fix bug in "inline double rho_pX" function
  3. replaced pow(somethig, 0,5) with sqrt
  4. replaced pow(somethig, 2) with something*something
  5. std::vector::push_back replaced with reserve + emplace_back
  6. some approaches to vectorize calculations in Region5::T_p(double p)
  7. added const modifier in constant methods
  8. some little optimizations

2) fix in "inline double rho_pX" function
3) replaced pow(somethig, 0,5) with sqrt
4) replaced pow(somethig, 2) with something*something
5) std::vector::push_back replaced with reserve + emplace_back
6) some approaches to vectorize calculations in Region5::T_p(double p)
7) added const modifier in constant methods
8) some little optimizations
@ibell
Copy link
Copy Markdown
Contributor

ibell commented Feb 19, 2017

Nice! Thanks for taking the time to put together this PR

@ibell ibell merged commit 5d2d294 into CoolProp:master Feb 19, 2017
@henningjp
Copy link
Copy Markdown
Contributor

@rafuck @ibell OK, for my own edification:

  1. Why is reserve + emplace_back better coding than std::vector::push_back? What does it achieve?
  2. What's the benefit of making all the base region functions const? This is a new concept for me.
  3. I get that sing PI = p/p_star, etc. creates another operation and variable and that p_star can be put into the interpolating equation directly, but this is how the IAPWS formulas are written. Is that small optimization better than maintaining consistency with the IAPWS documents (self-documenting)? Same thing with vectorization in Region5::T_p(double p) - doesn't this make it harder to read and debug for consistency with the IAPWS docs?

@rafuck
Copy link
Copy Markdown
Contributor Author

rafuck commented Feb 20, 2017

About 3) - now this function works 10x faster than initial func on my system.
About 1) and 2) I'll answer later

@rafuck
Copy link
Copy Markdown
Contributor Author

rafuck commented Feb 20, 2017

Ok, about const:

  1. https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#S-const
  2. If the compiler knows that the fields of a class instance are not modified across a const member function call, it doesn't have to reload any fields that it may have kept in registers before the const function call. Yes, on not so deep call tree compiler can poove, that some of object properties will not change, but if we say it explicity we can make compilation process little faster %)

About reserve+emplace vs push_back. It may be faster were size of vector is very big ( http://coliru.stacked-crooked.com/a/83d23c2d0dcee2ff ). But if the vector length is known in advance, why did not immediately allocate memory for it?

@henningjp
Copy link
Copy Markdown
Contributor

Excellent. Thanks. I'll read up!

@henningjp
Copy link
Copy Markdown
Contributor

@ibell Is CoolProp really off Python 2.7 now and can now use C++11 conventions? Is it worth putting the C++11 push_back code back in to add some speed?

@henningjp
Copy link
Copy Markdown
Contributor

@ibell - Bump.

@ibell
Copy link
Copy Markdown
Contributor

ibell commented May 27, 2024

I think that C++11 should be safe. In other projects I have moved to C++20.

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.

3 participants