fix #32

[marscher]

No description provided.

[franknoe]

This may be a stupid question since my C is extremely rusty, but doesn't this change require changing

if (_accept_step(log_prob_old, log_prob_new))

to

if (_accept_step(log_prob_old, log_prob_new) == 1)

or is 1 (here as shortint) automatically mapped to True and 0 to False in C?

[marscher]

Am 17.07.2015 um 20:40 schrieb Frank Noe:

This may be a stupid question since my C is extremely rusty, but doesn't
this change require changing

|if (_accept_step(log_prob_old, log_prob_new))|

to

|if (_accept_step(log_prob_old, log_prob_new) == 1)|

or is 1 (here as shortint) automatically mapped to True and 0 to False in C?

—
Reply to this email directly or view it on GitHub
#33 (comment).

My C is also a bit rusty, but I think int(0) is the only implicit way to
compare for truth. So maybe I just need to switch the type from short to
int.

[marscher]

actually it is the other way around...

[marscher]

this was a hard one. Finally it is fixed.

[franknoe]

Thanks a lot. But: Oh my god. Is it really that complicated? Is there no platform-independent way to check for infs or compute a min? Hard to believe.

Can we at least encapsulate these platform-dependent functions in separate functions and do the case switch in there? I don't to have them in the main code.

[marscher]

Am 22.07.2015 um 19:55 schrieb Frank Noe:

Thanks a lot. But: Oh my god. Is it really that complicated? Is there no
platform-independent way to check for infs or compute a min? Hard to
believe.
Really hard to believe is that a global player like Microsoft didnt
manage it in 15 years to implement C99 standard...

Can we at least encapsulate these platform-dependent functions in
separate functions and do the case switch in there? I don't to have them
in the main code.
maybe the author of the module should clean this up then. I was just
about fixing the builds for all the other people again.

—
Reply to this email directly or view it on GitHub
#33 (comment).

[franknoe]

OK, let me have a look.

Am 22/07/15 um 19:59 schrieb Martin K. Scherer:

Am 22.07.2015 um 19:55 schrieb Frank Noe:

Thanks a lot. But: Oh my god. Is it really that complicated? Is there no
platform-independent way to check for infs or compute a min? Hard to
believe.
Really hard to believe is that a global player like Microsoft didnt
manage it in 15 years to implement C99 standard...

Can we at least encapsulate these platform-dependent functions in
separate functions and do the case switch in there? I don't to have them
in the main code.
maybe the author of the module should clean this up then. I was just
about fixing the builds for all the other people again.

—
Reply to this email directly or view it on GitHub

#33 (comment).

—
Reply to this email directly or view it on GitHub
#33 (comment).

---

Prof. Dr. Frank Noe
Head of Computational Molecular Biology group
Freie Universitaet Berlin

Phone: (+49) (0)30 838 75354
Web: research.franknoe.de

Mail: Arnimallee 6, 14195 Berlin, Germany

[rmcgibbo]

C++ is your friend. If you want to support windows, C (C89) is essentially unusable. You could work around a lot of this with macros, but it's a loosing battle.

[franknoe]

Thanks for the hint. I would much prefer to use C++ anyway.

[marscher]

Its not about writing code in C++, but compiling the C code with the win C++ compiler (which is also pretty ugly).

[franknoe]

But then, why don't we just use the C++ compiler on Windows?

[rmcgibbo]

C++ is essentially a super set of C, so "writing C++" is really only as different as you want it to be. You can also just use the C++ on *nix too. MDTraj still uses a lot of C, but all of that was written before I realized the /better way/.

[marscher]

adding language='c++' to the extension and file extension cpp seems to work (in the sense, the M$ c++ compiler is invoked).

[franknoe]

Would this avoid the precompiler macros?

[franknoe]

@marscher, I don't think these two lines can be equivalent

    if(x >=eps && ! (!_finite(x) && !_isnan(x)))

    if (x >= eps && !isinf(x) && !isnan(x))

Since in the first one, one checks for finite, and in the second one for infinite. I suggest reverting this PR and finding a clean solution together with @trendelkampschroer tomorrow. Both his and my modules are affected, but I am essentially knocked out with meetings all day.

[trendelkampschroer]

I think renaming the function !_finite into _isinf, if possible would make things easier to read.

@marscher: Let's have a look at it today. Thanks for digging deep into the internals of the microsoft implementation of the C-standard library and coming up with a method to make this module compile also on Windows machines.

[franknoe]

Thanks! That one is not just about readability - the expression !_finite
is wrong. The line

    if (x >= eps && !isinf(x) && !isnan(x))

should translate to

    if (x >= eps && _finite(x) && !_isnan(x))

We definitely need stronger tests testing the result of a sampling. This
one is hard to catch as in most cases it would just end up sampling zero elements.

For readability and clarity of the code it would be good to redefine
functions such as isinf, isnan and fmin somewhere and then use them
throughout.

Am 23/07/15 um 09:26 schrieb Benjamin Trendelkamp-Schroer:

I think renaming the function !_finite into _isinf, if possible would
make things easier to read.

@marscher https://github.com/marscher: Let's have a look at it
today. Thanks for digging deep into the internals of the microsoft
implementation of the C-standard library and coming up with a method
to make this module compile also on Windows machines.

—
Reply to this email directly or view it on GitHub
#33 (comment).

---

Prof. Dr. Frank Noe
Head of Computational Molecular Biology group
Freie Universitaet Berlin

Phone: (+49) (0)30 838 75354
Web: research.franknoe.de

Mail: Arnimallee 6, 14195 Berlin, Germany