#include <stan/math/rev/scal/fun/inv.hpp>
#include <stan/math/rev/scal/fun/inv_sqrt.hpp>
#include <stan/math/rev/scal/fun/inv_square.hpp>
#include <stan/math/rev/scal/fun/log.hpp>  // gcc 4.9 cplx
#include <stan/math/rev/scal/fun/sin.hpp>  // gcc 4.9 cplx
#include <stan/math/rev/scal/fun/sqrt.hpp>
#include <stan/math/rev/scal/fun/square.hpp>
#include <stan/math/prim/scal/fun/pow.hpp>  // gcc 4.9 cplx
#include <stan/math/prim/scal/fun/is_nan.hpp>
#include <cmath>
#include <limits>

Here is what make test-headers is saying:
clang++-3.8 -Wall -I . -isystem lib/eigen_3.3.3 -isystem lib/boost_1.66.0 -isystem lib/sundials_3.1.0/include -std=c++1y -DBOOST_RESULT_OF_USE_TR1 -DBOOST_NO_DECLTYPE -DBOOST_DISABLE_ASSERTS -DBOOST_PHOENIX_NO_VARIADIC_EXPRESSION -Wno-unused-function -Wno-uninitialized -stdlib=libc++ -DNO_FPRINTF_OUTPUT -pipe  -c -O0 -include stan/math/rev/scal/fun/grad_inc_beta.hpp test/dummy.cpp -o /dev/null
In file included from <built-in>:351:
In file included from <command line>:6:
In file included from ./stan/math/rev/scal/fun/grad_inc_beta.hpp:12:
In file included from ./stan/math/rev/scal/fun/inc_beta.hpp:7:
In file included from ./stan/math/rev/scal/fun/pow.hpp:4:
In file included from ./stan/math/rev/cplx.hpp:5:
In file included from ./stan/math/rev/cplx/operator_division.hpp:6:
./stan/math/prim/cplx/operator_division.hpp:65:13: error: no matching function for call to 'pow'
  T const n(pow(u.real(), 2) + pow(u.imag(), 2));

Notice the include trace. The reason why the functions aren't being found is that the includes are being picked up from rev pow in the wrong order, rather than from a file like scal.hpp (which we can't use because it would make an include loop). When I initially laid out the complex class structure, I had to fight with this kind of thing a lot...
Note that if we want to fix one of the other functions that calls std::polar, such as exp, we will also be fighting a similar problem since it is likely that exp will need pow.
There is always a way out of include hell, we just have decide if it is worth the trouble...

Is it std::pow(double, double) that is a problem or only some complex versions?  If it's not just complex, then we need to take measures for Stan anyway.  should be doing this for Stan anyway.
Testing with fvar coupled with instantiating (not testing results) with fvar<fvar>, fvar, and fvar<fvar>.  The point is that if it works for fvar and instantiates elsewhere, the other cases derivatives should come for free from the rest of our autodiff implementation being right.
Is the proposal to put some new code in an ifdef for that compiler?  Is it complex-only or more general?  Either way's OK with me if pow() is broken.

It is the compiler combined with the complex header using it in a certain
way the breaks the correct order of function template overloading. It
manifests with the calls to "std::polar" (notice unfortunate namespace
prefix) inside complex, which happens to be triggered by calling pow on
data types that can shift type (say from zeroing to var). The bug
itself is very nasty, and manifests in many ways when attempting to work
around it.
On Mon, Jul 2, 2018, 7:48 PM Bob Carpenter<EMAIL_ADDRESS>wrote:

Is it std::pow(double, double) that is a problem or only some complex
versions? If it's not just complex, then we need to take measures for Stan
anyway. should be doing this for Stan anyway.
Testing with fvar coupled with instantiating (not testing results)
with fvar<fvar>, fvar, and fvar<fvar>. The point is that
if it works for fvar and instantiates elsewhere, the other cases
derivatives should come for free from the rest of our autodiff
implementation being right.
Is the proposal to put some new code in an ifdef for that compiler? Is it
complex-only or more general? Either way's OK with me if pow() is broken.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-401979499, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALTv-wMxgR6RllJOPxcHKBZ5vyh4NUj5ks5uCr95gaJpZM4ShF_6
.

To answer your question directly, it has no effect on notmal stan pow
operation
On Mon, Jul 2, 2018, 7:54 PM Chris Chiasson<EMAIL_ADDRESS>wrote:

It is the compiler combined with the complex header using it in a certain
way the breaks the correct order of function template overloading. It
manifests with the calls to "std::polar" (notice unfortunate namespace
prefix) inside complex, which happens to be triggered by calling pow on
data types that can shift type (say from zeroing to var). The bug
itself is very nasty, and manifests in many ways when attempting to work
around it.
On Mon, Jul 2, 2018, 7:48 PM Bob Carpenter<EMAIL_ADDRESS>wrote:

Is it std::pow(double, double) that is a problem or only some complex
versions? If it's not just complex, then we need to take measures for Stan
anyway. should be doing this for Stan anyway.
Testing with fvar coupled with instantiating (not testing
results) with fvar<fvar>, fvar, and fvar<fvar>. The point
is that if it works for fvar and instantiates elsewhere, the other
cases derivatives should come for free from the rest of our autodiff
implementation being right.
Is the proposal to put some new code in an ifdef for that compiler? Is it
complex-only or more general? Either way's OK with me if pow() is broken.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-401979499, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALTv-wMxgR6RllJOPxcHKBZ5vyh4NUj5ks5uCr95gaJpZM4ShF_6
.

Thanks!  Any word on when Rtools is moving off that compiler, Ben, or when we can move off Rtools for Windows?

On Jul 2, 2018, at 8:57 PM, ChrisChiasson<EMAIL_ADDRESS>wrote:
To answer your question directly, it has no effect on notmal stan pow
operation
On Mon, Jul 2, 2018, 7:54 PM Chris Chiasson<EMAIL_ADDRESS>wrote:

It is the compiler combined with the complex header using it in a certain
way the breaks the correct order of function template overloading. It
manifests with the calls to "std::polar" (notice unfortunate namespace
prefix) inside complex, which happens to be triggered by calling pow on
data types that can shift type (say from zeroing to var). The bug
itself is very nasty, and manifests in many ways when attempting to work
around it.
On Mon, Jul 2, 2018, 7:48 PM Bob Carpenter<EMAIL_ADDRESS>wrote:

Is it std::pow(double, double) that is a problem or only some complex
versions? If it's not just complex, then we need to take measures for Stan
anyway. should be doing this for Stan anyway.
Testing with fvar coupled with instantiating (not testing
results) with fvar<fvar>, fvar, and fvar<fvar>. The point
is that if it works for fvar and instantiates elsewhere, the other
cases derivatives should come for free from the rest of our autodiff
implementation being right.
Is the proposal to put some new code in an ifdef for that compiler? Is it
complex-only or more general? Either way's OK with me if pow() is broken.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-401979499, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALTv-wMxgR6RllJOPxcHKBZ5vyh4NUj5ks5uCr95gaJpZM4ShF_6
.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

No, but JJ was saying about a year from now.
@ChrisChiasson Do we know that we have to work around additional functions
besides pow?
On Mon, Jul 2, 2018 at 9:14 PM Bob Carpenter<EMAIL_ADDRESS>wrote:

Thanks! Any word on when Rtools is moving off that compiler, Ben, or when
we can move off Rtools for Windows?

On Jul 2, 2018, at 8:57 PM, ChrisChiasson<EMAIL_ADDRESS>wrote:
To answer your question directly, it has no effect on notmal stan pow
operation
On Mon, Jul 2, 2018, 7:54 PM Chris Chiasson<EMAIL_ADDRESS>wrote:

It is the compiler combined with the complex header using it in a
certain
way the breaks the correct order of function template overloading. It
manifests with the calls to "std::polar" (notice unfortunate namespace
prefix) inside complex, which happens to be triggered by calling pow on
data types that can shift type (say from zeroing to var). The bug
itself is very nasty, and manifests in many ways when attempting to
work
around it.
On Mon, Jul 2, 2018, 7:48 PM Bob Carpenter<EMAIL_ADDRESS>wrote:

Is it std::pow(double, double) that is a problem or only some complex
versions? If it's not just complex, then we need to take measures for
Stan
anyway. should be doing this for Stan anyway.
Testing with fvar coupled with instantiating (not testing
results) with fvar<fvar>, fvar, and fvar<fvar>. The
point
is that if it works for fvar and instantiates elsewhere, the
other
cases derivatives should come for free from the rest of our autodiff
implementation being right.
Is the proposal to put some new code in an ifdef for that compiler?
Is it
complex-only or more general? Either way's OK with me if pow() is
broken.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-401979499,
or mute
the thread
<
https://github.com/notifications/unsubscribe-auth/ALTv-wMxgR6RllJOPxcHKBZ5vyh4NUj5ks5uCr95gaJpZM4ShF_6

.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-401983213, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADOrqkrqOgcr5A4bDmyuRV0ySPZAdKZPks5uCsVYgaJpZM4ShF_6
.

Is that the time to the RStudio built-in version or the time to Rtools updating?

@bgoodri I am actually not sure the pow workaround itself passes make test-headers (at least not the way I did it -- and I am highly error prone), but there are about three calls to std::polar from other functions inside complex, but I didn't attempt to figure out if they trigger the same problem.

BTW, the rev scal fun exp_test has the problem also (exp is one of the functions that makes the call to std::polar), and its output can be seen in travis https://travis-ci.org/stan-dev/math/builds/399375207

OK, this is weird. The unit test at test/unit/math/rev/scal/fun/pow_test.cpp passes even after I get rid of the compiler guard. But make stan/math/rev/scal/fun/pow.hpp-test fails. I will figure it out.

Maybe check for possible differences in compiler and standard library
used... I am doing a lot of checking on that right now, including trying to
find a way to remove zeroing by having a single vari pre-initialized to
zero that can serve as the zero vari address for all vars. I wonder how to
make it work without breaking tests that like to check how many vars are on
the stack. I assume it means needing to override where it allocates itself.
By the way, the description of the nullaty ctor of var implies that the
nullary ctor (aka value initialization) of double is unusable until after
assignment. That is incorrect. double() is always zero. There is a
difference between double d(); and double d;
That is different than what var does.
On Tue, Jul 3, 2018, 2:03 PM bgoodri<EMAIL_ADDRESS>wrote:

OK, this is weird. The unit test at
test/unit/math/rev/scal/fun/pow_test.cpp passes even after I get rid of the
compiler guard. But make stan/math/rev/scal/fun/pow.hpp-test fails. I
will figure it out.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-402261544, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALTv-5X9UyaI03siP5K3RAPSExe7uAf1ks5uC7_8gaJpZM4ShF_6
.

OK, pow_test.cpp and exp_test.cpp are passing for me with g++-4.9, as is make test-headers. I am running the rest of ./runTests.py test/unit/math/rev now.
But I am getting an unrelated error (I think) from clang++-6.0.

Segmentation fault?

Nah, it is not that bad. Something about explicit specialization after initialization.

make stan/math/rev/core/matrix_vari.hpp-test
yields
clang++-6.0 -Wall -I . -isystem lib/eigen_3.3.3 -isystem lib/boost_1.66.0 -isystem lib/sundials_3.1.0/include -std=c++1y -DBOOST_RESULT_OF_USE_TR1 -DBOOST_NO_DECLTYPE -DBOOST_DISABLE_ASSERTS -DBOOST_PHOENIX_NO_VARIADIC_EXPRESSION -Wno-unused-function -Wno-uninitialized -stdlib=libc++ -DNO_FPRINTF_OUTPUT -pipe  -c -O0 -include stan/math/rev/core/matrix_vari.hpp test/dummy.cpp -o /dev/null
In file included from <built-in>:1:
In file included from ./stan/math/rev/core/matrix_vari.hpp:4:
In file included from ./stan/math/rev/mat/fun/Eigen_NumTraits.hpp:5:
In file included from ./stan/math/rev/core.hpp:49:
./stan/math/rev/core/std_numeric_limits.hpp:16:8: error: explicit specialization of 'std::__1::numeric_limits<stan::math::var>' after instantiation
struct numeric_limits<stan::math::var> {
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/c++/v1/math.h:471:44: note: implicit instantiation first required here
    std::is_arithmetic<_A1>::value && std::numeric_limits<_A1>::has_infinity,
                                           ^
1 error generated.

On the one you mentioned, I can't think of a reason why it would fail. One of the things I avoided by using std::complex on the existing types was not having to define numeric traits (and printing traits) for everything.

Yeah, I had to comment out that proj_test.cpp. But it should be easy enough to fix.

I wonder, is this file included in the one that is giving you the error? https://github.com/ChrisChiasson/math/blob/2039b4f34222aa09dbb03b69aa9900affd075053/stan/math/prim/mat/meta/Eigen_ScalarBinaryOpTraits.hpp

I do get a segfault with g++-8 from test/unit/math/rev/core/var_test.cpp . Here is the gdb session:
Program received signal SIGSEGV, Segmentation fault.
stan::math::var::val (this=0x7fffffffd7f8) at ./stan/math/rev/core/var.hpp:230
230       inline double val() const { return vi_->val_; }
(gdb) bt
#0  stan::math::var::val (this=0x7fffffffd7f8) at ./stan/math/rev/core/var.hpp:230
#1  0x000055555556d584 in stan::math::operator== (a=..., b=...) at ./stan/math/rev/core/operator_equal.hpp:27
#2  0x0000555555576405 in std::__complex_abs<stan::math::zeroing<stan::math::var> > (__z=...) at /usr/include/c++/8/complex:598
#3  0x0000555555571fbd in std::abs<stan::math::var> (__z=...) at /usr/include/c++/8/complex:618
#4  0x0000555555567137 in AgradRev_complexNotNullIssue123_Test::TestBody (this=0x555555844bd0) at test/unit/math/rev/core/var_test.cpp:85
#5  0x00005555555d72ca in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ()
#6  0x00005555555ceabd in testing::Test::Run() [clone .part.451] ()
#7  0x00005555555cefd5 in testing::TestInfo::Run() [clone .part.452] ()
#8  0x00005555555cf16d in testing::TestCase::Run() [clone .part.453] ()
#9  0x00005555555d2c2d in testing::internal::UnitTestImpl::RunAllTests() [clone .part.478] ()
#10 0x00005555555d2ea5 in testing::UnitTest::Run() ()
#11 0x00005555555baaad in RUN_ALL_TESTS () at lib/gtest_1.7.0/include/gtest/gtest.h:2288
#12 0x00005555555baa3b in main (argc=1, argv=0x7fffffffdeb8) at lib/gtest_1.7.0/src/gtest_main.cc:48

That numeric limits need to be included before another piece of code instantiates the default base template class.
Specifically, the numeric_limits specialization needs to come before the instantiations of std::complex and Eigen::Matrix<var,...>.
For efficiency within Eigen, we need to specialize for
std::numeric_limits<std::complexstan::math::var>
and make sure that also gets included first.

I do believe I know the reason for the segault, because it is the same as what happens if clang uses lidstdc++. Within that method std::absstan::math::var(some argument), it calls _Tp(). However, _Tp is stan::math::var. I believe the reason it doesn't show up in current gcc is the super-weird bug. The fix is to create a new overload for abs that has a tighter template overload for abs(complex<fvar>) (so it will invoke with a higher priority), and a plain overload for abs(complex). Alternatively, I could fix the fundamental problem with var.

Well, the bug that allows it to work must be present in g++-5 because the rev tests are passing there. Let me push what I have.

I think I agree. The stack overflow thread you linked had a link to a g++ mailing list. In the mailing list message, the commit they wanted to push as the fix just altered the standard library to explicitly call std::polar(r,theta) rather than std::polar(r,theta) (i.e. template parameter was not required to be deduced anymore). I had sent this via email about 10 min ago but it didn't show up.

Sketch of what I am going to experiment with to remove zeroing and eliminate problems in complex... The idea is to point a defaulted zero var to a pre-existing instance to keep speed.
This is just an experiment. I understand it has a high barrier to entry even if it worked and performed perfectly.
// autodiffstackstorage.hpp
template <class T>
struct ZeroStatic {
 /* T() is private, so init_type is necessary
  * because non-const static data member
  * initializers aren't allowed */
 static T init_type() {return T();}
 static T& zero() {
#ifdef STAN_THREADS
    thread_local static T zero_(init_type());
#endif
    return zero_;
  }
#ifndef STAN_THREADS
 private:
  static T zero_;
#endif
};

#ifndef STAN_THREADS
template <class T>
T ZeroStatic<T>::zero_(ZeroStatic<T>::init_type());
#endif

// vari.hpp

class vari : private ZeroStatic<vari> {
friend ZeroStatic<vari>;
public:
 double v;
 ...
private:
 // *may* be necessary b/c of private inheritance; will check...
 // var is a friend of vari, but I don't know how deep it can reach
 using ZeroStatic<vari>::zero();

 vari(): v(0), ... {}
};

// var.hpp
 ...
 // the pre-existing note on this ctor implies that double() isn't zero...
 // there is a difference between double d(); and double d; ... only the latter isn't always zero...
 // this is the source of the problems in complex, because it wants var() to actually be zero
 var() : vi_(&zero());
 ...
 // will have to see what to do with is_uninitialized

May be able to constexpr this too
On Wed, Jul 4, 2018 at 2:10 AM ChrisChiasson<EMAIL_ADDRESS>wrote:

Sketch of what I am going to experiment with to remove zeroing and
eliminate problems in complex... The idea is to point a defaulted zero var
to a pre-existing instance to keep speed.
This is just an experiment. I understand it has a high barrier to entry
even if it worked and performed perfectly.
// autodiffstackstorage.hpp
template
struct ZeroStatic {
/* T() is private, so init_type is necessary

because non-const static data member
initializers aren't allowed */
static T init_type() {return T();}
static T& zero() {
#ifdef STAN_THREADS
thread_local static T zero_(init_type());
#endif
return zero_;
}
#ifndef STAN_THREADS
private:
static T zero_;
#endif
};

#ifndef STAN_THREADS
template
T ZeroStatic::zero_(ZeroStatic::init_type());
#endif
// vari.hpp
class vari : private ZeroStatic {
friend ZeroStatic;
public:
double v;
...
private:
// may be necessary b/c of private inheritance; will check...
// var is a friend of vari, but I don't know how deep it can reach
using ZeroStatic::zero();
vari(): v(0), ... {}
};
// var.hpp
...
// the pre-existing note on this ctor implies that double() isn't zero...
// there is a difference between double d(); and double d; ... only the latter isn't always zero...
// this is the source of the problems in complex, because it wants var() to actually be zero
var() : vi_(&zero());
...
// will have to see what to do with is_uninitialized
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-402373690, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADOrqhQTbIzNUP8veoINN3WC2rpXbmXLks5uDFxbgaJpZM4ShF_6
.

Experimentation is great, but as far as I can tell, we are down to fixing proj and maybe doing some overloads for fvar<var> like I did in the var case for exp and pow. If we could achieve that, get it reviewed from all angles, and merge it, then it could get included in 2.18 and we could experiment with refactorings for 2.19.

ok I will try to fix abs today after I take care of something for a coworker...

I don't know if you've seen @bob_carpenter's last comment, but that needs
to happen. I think a lot of issues are stemming from not instantiating
template metaprograms in the correct order.
Please look at the order of the first includes in headers like
stan/math/rev/mat.hpp. Those first few headers MUST BE in that order or
else things won't work in the same way.
When you instantiate std::complex, I don't know if this has to be before or
after Stan's instantiation of Eigen NumTraits to work properly.
On Thu, Jul 05, 2018 at 9:18 AM<EMAIL_ADDRESS>wrote:

ok I will try to fix abs today after I take care of something for a
coworker...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-402718726, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAZ_F_vbSMScEiBg3kyS1j7ZoORuA0XSks5uDhIegaJpZM4ShF_6
.

I am running into problems with the integrator while trying to test my actual complex code. Something about y0, check_finite, and stan::get. I think I can tell what is happening. Somehow the overload for stan::get on vectors isn't being called, and is falling back to the one for primitive types, which is a no-op that returns its argument. So, check_finite is saying that it doesn't know what to do with a std::vector. I tried bringing in the array version of stan::get into the rk45 integrator file, but it didn't help. So, I am attempting to rebase onto develop to see if that works.

This is related to that abs () commit somehow? Which compiler was that not
working with?
On Jul 5, 2018 6:55 PM, "ChrisChiasson"<EMAIL_ADDRESS>wrote:

I am running into problems with the integrator while trying to test my
actual complex code. Something about y0, check_finite, and stan::get. I
think I can tell what is happening. Somehow the overload for stan::get on
vectors isn't being called, and is falling back to the one for primitive
types, which is a no-op that returns its argument. So, check_finite is
saying that it doesn't know what to do with a std::vector. I tried bringing
in the array version of stan::get into the rk45 integrator file, but it
didn't help. So, I am attempting to rebase onto develop to see if that
works.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-402875584, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADOrqvUCQ-tlsedojSa3-p72x5A7PYP2ks5uDpl5gaJpZM4ShF_6
.

This is maybe not helpful but sometimes when I've seen that it's because I'm not including the right include - prim/scal.hpp instead of prim/mat.hpp for example.

After the rebase onto develop, it looks like clang++-3.8 may be using libstdc++ and choking on it (as opposed to libc++).  Later clangs handle it better... what to do?

@seantalts for the integrator issue, I had a math file in my project that loaded the mix scalar. The project did include the relevant overall mix matrix file, but that was afterward. I had to change my own math file to not use the mix scalar at all.

I'm not seeing the clang++-3.8 thing you're talking about - the last error was on a Mac, here: http://d1m1s1b1.stat.columbia.edu:8080/blue/organizations/jenkins/Math Pipeline/detail/PR-789/98/pipeline

I'm lost. What problem was the abs overload intended to solve and has it been solved?

It was needed if someone issued something like
abs(std::complex(1,1)) using libstdc++ on current gcc or clang. This
would deduce to std::abs(std::complex). At that point, var()
would get called.
The overloads intercept and force a call to
abs<zeroing>(std::complex). Then zeroing() gets called, which is
ok. So it appears those overloads are working now, which is good because
that is how a lot of people will use the library (modern gcc or clang with
libstdc++).
I am still working on my own application. Still have some problems, but I
am working through them.
On Tue, Jul 10, 2018, 7:00 PM bgoodri<EMAIL_ADDRESS>wrote:

I'm lost. What problem was the abs overload intended to solve and has it
been solved?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-404002495, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALTv-8kWKdwCu_aMWTG0CG2t0x4-hFCpks5uFUAEgaJpZM4ShF_6
.

That reminds me. I need to make sure the fvar version won't attempt to call
itself.
On Tue, Jul 10, 2018, 7:10 PM Chris Chiasson<EMAIL_ADDRESS>wrote:

It was needed if someone issued something like
abs(std::complex(1,1)) using libstdc++ on current gcc or clang. This
would deduce to std::abs(std::complex). At that point, var()
would get called.
The overloads intercept and force a call to
abs<zeroing>(std::complex). Then zeroing() gets called, which is
ok. So it appears those overloads are working now, which is good because
that is how a lot of people will use the library (modern gcc or clang with
libstdc++).
I am still working on my own application. Still have some problems, but I
am working through them.
On Tue, Jul 10, 2018, 7:00 PM bgoodri<EMAIL_ADDRESS>wrote:

I'm lost. What problem was the abs overload intended to solve and has it
been solved?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-404002495, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALTv-8kWKdwCu_aMWTG0CG2t0x4-hFCpks5uFUAEgaJpZM4ShF_6
.

I think we should replace gcc's proj with this implementation from libc++ IF we really want it to match the @bgoodri mentioned from cpp reference. The reason why is that, as @bgoodri quoted in the g++ source comment, the additional function was meant to resolve LWG DR 781. That LWG issue calls out that the function should return the same results as the corresponding C functions. G++ appears to be trying to reproduce a continuous mapping in the mathematical sense, whereas clang is making a hard switch at any of the infinities. If we don't think the handling at the complex poles should diverge from what g++ uses for the double and floating point types, then we should leave it alone. In this case libstdc++ has only one template that is used for all types. There are no specializations for double, float, etc. It also suggests a method of testing if their c++ library agrees with their c library... See what g++ returns when cproj is called instead of std::proj.

What is happening is that  _GLIBCXX_USE_C99_COMPLEX = 1. When
template<typename _Tp>
    inline std::complex<_Tp>
    proj(const std::complex<_Tp>& __z)
    { return __complex_proj(__z.__rep()); }

is called with _Tp = double, then the below overload of __complex_proj takes over. In other words, I was wrong that the template was the only version active. It would have been if the macro were undefined.
  inline __complex__ double
  __complex_proj(__complex__ double __z)
  { return __builtin_cproj(__z); }

As we can see, cproj is called on the C99 representation of the complex number. This is why the results are different. I have verified it by deleting the overload and also the call to the __rep() method and comparing the output. In that case, the program above produces identical output with double and with var:
proj(1,2) = (0.333333,0.666667)
proj(inf,-1) = (nan,-0)
proj(0,-inf) = (0,nan)
proj(1,2) = (0.333333,0.666667)
proj(inf,-1) = (nan,-0)
proj(0,-inf) = (0,nan)

How can I restart Jenkins without pushing a commit? It just keeled over for no reason.

Sorry, I'm currently adding a new machine and there was a hiccup. I restarted it. There's a little replay button that looks like an arrow made into a circle in the top right if you're logged in.

Hrm. Jenkins just wrote a somewhat concerning commit just now. It is touching many different parts of the system.

Thanks for the heads up. I’ll look into it - meanwhile feel free to force
push over it.
On Thu, Jul 12, 2018 at 16:31 ChrisChiasson<EMAIL_ADDRESS>wrote:

Hrm. Jenkins just wrote a somewhat concerning commit just now. It is
touching many different parts of the system.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-404641047, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAxJ7BATkhqHroBuv_BgFISZSU-MiNBcks5uF7IogaJpZM4ShF_6
.

--
Laboriously extracted via diminutive keyboard

I think it will be okay

I logged into Jenkins via github's oauth, but I don't see the restart check (maybe I don't have permission?). I am on my phone (with "request desktop site") because my corporate laptop can't reach Jenkins. I am tapping all around inside of jenkins (not just the first status page). I did find this plugin online, but the relevant menu entry isn't there after preferences, so it is either not the right plugin or doesn't apply to this situation because the build is in progress. I am attempting to restart now rather than waiting because the previous build (before the manual restart by Sean) stalled at this stage for an hour before failing.

I get a circle with a stop button in it when I log in. That's where the restart icon usually is.

I hit that button. It now says it is waiting for Windows or Linux. There
are a few other PRs being tested ATM.
On Thu, Jul 12, 2018 at 5:18 PM Ben Bales<EMAIL_ADDRESS>wrote:

[image: jenkins]
https://user-images.githubusercontent.com/4742424/42659834-80a9dca0-85f7-11e8-8c4f-636a263248d8.png
I get a circle with a stop button in it when I log in. That's where the
restart icon usually is.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-404653815, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADOrqv99rW02xnfNfdmIdZBW4OK3pknVks5uF70WgaJpZM4ShF_6
.

Now the status is terminated. I don't have the button pictured above. It is possible that jenkins is detecting the phone user agent (it shows nearly the same thing whether or not request desktop site it marked). I only have the pencil icon, no gear or start/stop. The pencil icon gives  a permissions error when I hit it.

I'll attempt to solve this tomorrow... I fixed a similar problem back with my initial implementation (the hackish one that didn't fit stan's layout), but I don't remember how.
test/unit/math/fwd/mat/fun/dims_test.cpp:74:6: error: call to member function 'push_back' is ambiguous

  x3.push_back(-32.1);

  ~~~^~~~~~~~~

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/vector:700:36: note: candidate function

    _LIBCPP_INLINE_VISIBILITY void push_back(const_reference __x);

                                   ^

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/vector:702:36: note: candidate function

    _LIBCPP_INLINE_VISIBILITY void push_back(value_type&& __x);

                                   ^

test/unit/math/fwd/mat/fun/dims_test.cpp:75:6: error: call to member function 'push_back' is ambiguous

  x3.push_back(17.9);

  ~~~^~~~~~~~~

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/vector:700:36: note: candidate function

    _LIBCPP_INLINE_VISIBILITY void push_back(const_reference __x);

                                   ^

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/vector:702:36: note: candidate function

    _LIBCPP_INLINE_VISIBILITY void push_back(value_type&& __x);

                                   ^

2 errors generated.

make: *** [test/unit/math/fwd/mat/fun/dims_test.o] Error 1

So I'm setting up a new Jenkins agent and got the clang-format version on it wrong (apparently we need version 5.0 if it's ubuntu, 6.0 if it's on a Mac, hah). Fixed this on develop too. Nothing was broken, but there were a couple of new fixes in 6.0-1ubuntu2 or whatever version that was. Feel free to revert or remove the auto-format commit from Jenkins clang-format 6.0.0-1ubuntu2 (65be731).

At least travis is all green again https://travis-ci.org/stan-dev/math/builds/403337444?utm_source=github_status&utm_medium=notification

BTW, I don't have access to a mac, so if anyone already knows the answer that would be much appreciated

None of the linux VMs I have access to are throwing the error with ~/stan-dev/math$ ./runTests.py test/unit/math/fwd/mat/fun/dims_test. Works on clang++3.8 with libstdc++ and libc++. Works on g++-4.9. Works on clang++6.0 with libstdc++ and lic++. Works on g++-7.3. Yes, I did make clean between runs. Going to attempt to read the code to see if I can figure it out.

Hey @ChrisChiasson, we talked about changing our officially supported compilers at the meeting yesterday but I haven't had time to write up a discourse post. Long story short, we're probably going to stop testing on Apple's clang and switching to testing just on default installs of clang on Linux and Mac (based on the version from clang's website). So this problem may go away in the next week or so - currently I'm wrapped up doing the 2.18 release.

Jenkins: "epyc is offline; gelman-group-win2 is reserved for jobs with matching label expression"

Yep it's queued.
On Fri, Jul 13, 2018 at 12:41 ChrisChiasson<EMAIL_ADDRESS>wrote:

Jenkins: "epyc is offline; gelman-group-win2 is reserved for jobs with
matching label expression"
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-404887788, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAxJ7M9i9aSEoj2nP_r7FckVqPrU2btbks5uGM21gaJpZM4ShF_6
.

--
Laboriously extracted via diminutive keyboard

Thanks, wasn't sure if it was failed or just delayed.
On Travis, one of the sub builds failed / didn't start:
"OCI runtime exec failed: exec failed: container_linux.go:348: starting container process caused "could not create session key: disk quota exceeded": unknown"

I'm literally going to remove Travis entirety in the next week.
On Fri, Jul 13, 2018 at 14:10 ChrisChiasson<EMAIL_ADDRESS>wrote:

Thanks, wasn't sure if it was failed or just delayed.
On Travis, one of the sub builds failed / didn't start:
"OCI runtime exec failed: exec failed: container_linux.go:348: starting
container process caused "could not create session key: disk quota
exceeded": unknown"
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-404911075, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAxJ7AFDYKSQRwAQJzowIT29HwLeA06Fks5uGOKKgaJpZM4ShF_6
.

--
Laboriously extracted via diminutive keyboard

Ok, I am going to roll back the last commit because this Jenkins Xcode error is scarier and happens earlier in the pipeline than the last one. Looking forward to next week. Have a good weekend y'all :)

Holding off for issue #943 and PR #944 to go through before continuing

since #944 went through, I have rebased onto develop

@bgoodri Jenkins is complaining about a pseudo eigendecomposition test difference from zero being greater than a tolerance. The actual differences are on the order of 1E-12. This is from test/unit/math/rev/mat/fun/pseudo_eigendecomposition_test.cpp line 45. I remember you and @syclik had some discussion regarding this, but I don't know if it's relevant.

Yeah, we should just relax TOL.
On Wed, Jul 25, 2018 at 5:46 PM ChrisChiasson<EMAIL_ADDRESS>wrote:

@bgoodri https://github.com/bgoodri Jenkins is complaining about a
pseudo eigendecomposition test difference from zero being greater than a
tolerance. The actual differences are on the order of 1E-12. This is from
test/unit/math/rev/mat/fun/pseudo_eigendecomposition_test.cpp line 45. I
remember you and @syclik https://github.com/syclik had some discussion
regarding this, but I don't know if it's relevant.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-407907052, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADOrqusD-7vj203ZaXDcQCG_Hsvdc_w1ks5uKOczgaJpZM4ShF_6
.

Some relevant information:

Google Test has updated doc that's easier to read now
Google Test's Advanced Guide explains the macros for floating point comparison
The advanced guide links to this post, Comparing Floating Point Numbers, 2012 Edition, that gets into comparing floating point numbers.

@bob-carpenter @syclik ready for review again; all checks passed

@ChrisChiasson, as far as I can tell, this isn't ready for review. I still don't know what you're trying to do!
Can you write up a spec for what you're trying to accomplish and put it in the associated issue? If you have the ability, updating the first comment would be great.
Then, can you write some notes to guide the reviewer through this implementation against that spec that you describe? It'd help to know some of the major design decisions and whether you've implemented the whole spec or part of it and there's more remaining.

@syclik the issue was that complex var numbers couldn't be used because it would automatically dereference a null ptr of thw imaginaty part. All this does is fix it so that complex numbers can be used. "Used" is defined at the top of this PR. It has also gone beyond that to implement the C++14 list of functions that work on complex numbers, thanks to @bgoodri adding many tests. For now I am considering the top post of this PR to be the location where the spec is listed, because that is where a previous reviewer asked me to put it (@bob-carpenter ).I will be happy to add the C++14 note to it if you like. Van you classify "major design decision"?

I'll just start with the first post: I don't understand well enough to
understand the usage.
I think you think the quoted paragraph explains it. Can you rewrite it so
that it has more context and more details? There still isn't enough there
to determine whether we want to accept this into Math. If you asked me to
evaluate this now, I'd have to reject because it adds maintenance burden
and the use isn't clear. I'm not suggesting that happen, I'm suggesting
strongly that the top level comment have enough context and detail to
understand what's being done. If you need help with this, please ask. I
think the best person to help would be @bgoodri. Once it's in, the code is
a community resource that's maintained, so it is a big deal that we
understand why this is happening.
Hope that makes sense.
I haven't looked at the code, but things that are decisions are things like
how you came to the conclusion the traits needed to be implemented and
included in the order you've included it. If you've changed constructors,
why that happened. I know... it's vague, but what I'm asking for is for
you, as the contributor, to leave breadcrumbs for the reviewer to follow.
This PR has 142 files changed! Where should the reviewer start? I just
peeked... why is there a new folder at the {scal, arr, mat} level? These
are things you should help a reviewer with.
On Fri, Jul 27, 2018 at 8:05 AM ChrisChiasson<EMAIL_ADDRESS>wrote:

@syclik https://github.com/syclik the issue was that complex var
numbers couldn't be used because it would automatically dereference a null
ptr of thw imaginaty part. All this does is fix it so that complex numbers
can be used. "Used" is defined at the top of this PR. It has also gone
beyond that to implement the C++14 list of functions that work on complex
numbers, thanks to @bgoodri https://github.com/bgoodri adding many
tests. For now I am considering the top post of this PR to be the location
where the spec is listed, because that is where a previous reviewer asked
me to put it (@bob-carpenter https://github.com/bob-carpenter ).I will
be happy to add the C++14 note to it if you like. Van you classify "major
design decision"?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-408399420, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAZ_F2WjG-y5BNHiiBiJ-GrFOKvmn39Oks5uKwHwgaJpZM4ShF_6
.

@syclik first post updated per your comments

The use of the PR should be clear. If Stan Math <= 2.18 is an autodiff library for real numbers, then with this PR, Stan Math becomes an autodiff library for complex numbers conceptualized as a pair of var (or fvar<T>) one for the real part and one for the imaginary part. If z = a + b * i is a stan::math::var, then stan::math::foo(z) should create valid nodes on the autodiff tree, where foo is any function defined by the C++14 standard, such as log.
There are several situations in which you would want to autodiff functions involving complex numbers, but none of them are actually implemented in this PR, which is fine because this PR is just the framework for them. Future PRs could implement FFTs, various Eigen functions that utilize complex numbers internally, PDFs for complex random variables, etc. We should not wait on this PR for things like that.
@syclik your questions are largely about the implementation decisions. I don't fully understand why myself, but I do know that everything before this PR that pertained to complex numbers either would not compile or would segfault as soon as a complex var is instantiated.  The main challenge is that the default constructor for a std::complex<T> is only well-defined when T is float, double, long double, etc. and it segfaults when T is a var. So, the guiding principle of the stuff in the cplx subfolder is to circumvent calling the default constructor, and it is quite possible that this PR is the only (known) feasible way to generalize Stan Math to be an autodiff library for complex numbers.
The second principle is that functions in Stan Math under rev and fwd need to get called on the real and imaginary parts of a std::complex<stan::math::var>.  So, there is a bunch of stuff to ensure that happens, as opposed than calling functions in std or some other namespace.
But there are not many functions whose signature is a std::complex<stan::math::var> or similar. There are a few because even though C++11 defines a bunch of functions involving complex numbers that the standard library must implement, the implementations of them were not that good in earlier versions of stdlibc++ and libc++. With g++-4.9, there are several functions in the standard library that call other functions in the standard library but prefix them with std:: instead of allowing specializations to take precedence. So, there is some confusing stuff where we avoid calling functions in the standard libraries and basically reimplement what the standard libraries do or should do for those functions.

@syclik The cplx folder exists parallel to {scal, arr, mat} because this was the least intrusive way I could figure out to integrate complex math into the existing math hierarchy. In general, complex's functionality depends on having complete definitions of the data type in scal, and, indeed, similar to an array, it contains more than one of the scalar data type it references. Without this kind of organization, it is surprisingly easy (and painful) to get oneself into a situation where a definition will be needed that isn't available without making a circular include. The best analogy I could think of is what would happen if someone tried to re-write the standard library complex class without a complete definition of double already available...

@syclik first post updated per your comments

Awesome. Thanks!

The use of the PR should be clear. If Stan Math <= 2.18 is an autodiff library for real numbers, then with this PR, Stan Math becomes an autodiff library for complex numbers conceptualized as a pair of var (or fvar) one for the real part and one for the imaginary part. If z = a + b * i is a std::complexstan::math::var, then stan::math::foo(z) should create valid nodes on the autodiff tree, where foo is any function defined by the C++14 standard, such as log.
There are several situations in which you would want to autodiff functions involving complex numbers, but none of them are actually implemented in this PR, which is fine because this PR is just the framework for them. Future PRs could implement FFTs, various Eigen functions that utilize complex numbers internally, PDFs for complex random variables, etc. We should not wait on this PR for things like that.

Thanks! That's the clearest I've seen it written out. Just to be clear, we never want to take a derivative of a std::complex<stan::math::var>, correct? We only want to take derivatives of the real or imaginary part (but never both)?
Thanks both for clarifying the issue. That explains what we want.
Time to get to the implementation and whether this is a decent implementation that gets us there. I'll start reviewing the code.

Bob attempted to answer the real vs imaginary part question in one of his last replies on discourse.  To me this means the derivative of both components is taken.

Mind merging develop back in? It looks like some of the diff isn't related to the issue at hand and it's a little distracting.

 To me this means the derivative of both components is taken.

We're not using the same terminology. If x = f(theta) and y = g(theta), then when you say you want the derivative for both components, what I understand that to mean is to have both:

dx / dtheta = f'(theta)
dy / dtheta = g'(theta)

Instead, what you're returning is just one term (when theta is scalar):

d (x + constant * y) / dtheta = f'(theta)  + constant * g'(theta)

I just don't read that to mean the "derivative of both components is taken"; hope that clears up a lot of the confusion in this thread and on discourse. I now understand that what you mean and how that plays out, so it's a lot clearer.
I'll try to review this, but I may have a lot of questions for you.

@syclik I don't really understand what you're getting at. If you make an example of what you're talking about with a size 2 std::vector for taking derivatives with respect to one, both, or neither components, I will translate it to the analogous complex case for you.

@ChrisChiasson, did you go through all the implementation notes here: https://en.cppreference.com/w/cpp/numeric/complex? (that's a question... I haven't verified that you did or didn't)
There's no discussion in this pull request regarding what parts of it you have or haven't implemented.

@syclik I have read the page before. Is there some part of it you would like me to comment on?

@syclik for the merge, I don't think it would help at all. The entire set of commits was rebased onto develop right before my review request.

@ChrisChiasson, maybe this would explain it?
#include <stan/math.hpp>
#include <iostream>

int main() {
  std::cout << "compute dx[0] / dtheta" << std::endl;

  std::vector<stan::math::var> x;
  std::vector<double> gradient;

  stan::math::var theta = 0.5;
  std::vector<stan::math::var> vars;
  vars.push_back(theta);

  x.push_back(sin(theta));
  x.push_back(cos(theta));

  std::cout << x[0] << std::endl
            << x[1] << std::endl
            << std::endl;

  x[0].grad(vars, gradient);

  std::cout << "gradient.size() " << gradient.size() << std::endl
            << "dx[0] / dtheta = " << gradient[0] << std::endl;

  // clear everything
  stan::math::recover_memory();
  x.clear();
  vars.clear();

  // d x[1] / dtheta
  std::cout << "compute dx[1] / dtheta" << std::endl;

  theta = 0.5;
  vars.push_back(theta);
  x.push_back(sin(theta));
  x.push_back(cos(theta));

  std::cout << x[0] << std::endl
            << x[1] << std::endl
            << std::endl;

  x[1].grad(vars, gradient);

  std::cout << "gradient.size() " << gradient.size() << std::endl
            << "dx[1] / dtheta = " << gradient[0] << std::endl
            << std::endl;

  // clear everything
  stan::math::recover_memory();
  x.clear();
  vars.clear();

  // compute both d x[0] / dtheta and d x[1] / dtheta simultaneously
  std::cout << "can't compute both dx[0] / dtheta and dx[1] / dtheta simultaneously" << std::endl;

  return 0;
}

The entire set of commits was rebased onto develop right before my review request.

Ok. Can you revert the changes you've made to the makefiles?

@syclik yes, but if I revert that change, that will make the file different from develop

Is there some part of it you would like me to comment on?

Sure. All of it? Did you satisfy all the requirements? How did you validate that std::complex<stan::math::___> is a LiteralType? Did you check that the the real and imaginary parts are sitting next to each other in memory?
There's a lot there... I don't know how much you walked through it or didn't.

re: make. ok. I have no idea what GitHub is doing.

@syclik It is unlikely that we'll satisfy all the requirements. In the first instance, adding the types of definitions we have to standard namespace are not guaranteed by the standard (i.e. the minute we add function like std::abs or a type like std::complex, the standard says the compiler is free to dump us in undefined behavior land).

@ChrisChiasson, yes, I read that.
Questions that would help everyone else understand what you're doing:

what parts of the (soft) requirements are you implementing?
what parts are you explicitly not implementing?
why? (to both)
did you add tests that checked those things?

In a pragmatic sense, things that are undefined are ok as long as we know that the compiler does the right thing (it looks like they start allowing it for C++20). If we have tests in place, at least we can validate that we're ok even if we're in undefined behavior land.

@syclik After reading the program listing var.grad(vector<var>,vector<double>) .... That is more of a limitation of the grad method of var than a complex<var> implementation. It doesn't really reflect the math notation used above either. It would make more sense to use fvar here. If the question is instead: "Did you add a .grad() method to std::complex objects?" (which is what would be required) the answer is "I did not"

re: contiguous. Awesome. I didn't realize that was going to be implemented by the standard library. It'd just help line up these things.
re: limitation of grad. Hopefully that clears it up for you why I was asking for what you were attempting to do. That's not a limitation in our design: it's a limitation of reverse-mode automatic differentiation when it can't be retaped. Also, I don't understand what it means to compute .grad() on std::complex -- I'm not sure what that would return. Anyway, I understand that it's not what we're trying to do, so let's focus on helping me (or any other reviewer) understand what you're accomplishing and how.

@syclik please let me know what else needs to be explained and changed for this review

@ChrisChiasson, I've been staring at the code. I just made some minor-ish comments (been sitting there as I tried to make heads / tails of the code).
Some bigger questions that don't fit into a line-by-line commentary:

Why is there a stan::math::complex<T> type defined?
Instead of having the zeroing construction, would it be possible to just write the instantiation so it calls the constructor with 0? Is it difficult to completely template instantiate the class? Are you trying to avoid that?
Can you explain the side effect a little more?

Sorry it's taken a while. Here's what would help me (or anyone else) review it:

a guide to how to navigate your code.
any design decisions you made: why zeroing?
what functions are implemented in the std namespace and why
what functions are implemented in math? (just to have a list)

I haven't walked through the tests yet. It'd help to know what you were trying to test. If not, it'll take a while to walk through and understand what you're trying to test.

@syclik I answered your code comments/questions in-line. They also answer question 1.
For question 2, the codes that call T() are in the standard libraries, so we don't get a choice.
For question 3, I am not sure what side effect you are referring to, but if you are talking about zeroing<T>, the side effect is that the code would try to dereference the null pointer generated inside var() or fvar<var>().
I'll be your personal guide for navigation... Writing a guide to navigate stuff inside stan math would be more complicated than writing this whole PR...
For "why zeroing" I explained that in the inline comments.
The only functions I've implemented in the std namespace are whichever ones forced the issue. Usually by having a call from the standard library that was prefixed with std or that were called unprefixed in a way that would force resolution to the std version (e.g. the argument is std::complex<T>). In those cases, when the std function would fail to compile, sometimes due to standard library bugs, I have to provide a tightly scoped overload within the std namespace. I didn't maintain a list of functions where I had to provide std overloads.
I don't have a list of functions implemented in the stan::math namespace.
The closest approximations to these types of lists are to look at the changes to core.hpp, scal.hpp, mat.hpp, and so on.
Regarding testing: I have provided one major test and two minor tests. The major test is a double, var and fvar<var> test that constant checks the validity of eigendecomposition of a rotation matrix, where newton's method is used to drive the complex and imaginary parts of the first eigenvalue of the rotation to equal each other. It is located here https://github.com/stan-dev/math/blob/07ffd6c0dde3ed919248a4d43cd80bba066ceeba/test/unit/math/mix/mat/functor/autodiff_test.cpp#L298 . The two minor tests illustrate the basic functionality for complex differentiation. They have several comments to indicate what is going on, and they mirror each other: https://github.com/stan-dev/math/blob/07ffd6c0dde3ed919248a4d43cd80bba066ceeba/test/unit/math/fwd/scal/fun/abs_test.cpp#L52 and https://github.com/stan-dev/math/blob/07ffd6c0dde3ed919248a4d43cd80bba066ceeba/test/unit/math/rev/core/var_test.cpp#L74
Later, @bgoodri added many more.

Are you saying you want to move isinf from stan::math to std? What is the
full rationale?
On Mon, Aug 6, 2018, 7:31 PM Daniel Lee<EMAIL_ADDRESS>wrote:

@syclik commented on this pull request.
In stan/math/fwd/scal/fun/is_inf.hpp
https://github.com/stan-dev/math/pull/789#discussion_r208067179:

@@ -20,6 +20,12 @@ inline int is_inf(const fvar& x) {
return is_inf(x.val());
}

+// forwarding for ADL
+template
+inline auto isinf(const fvar& a) {
I think we're way past that; std::complex for our types are unspecified.
We should do what we can verify is correct, is maintainable, and makes
sense.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#discussion_r208067179, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALTv-9_OhTM7DJH6mjUBhryUkGyWX4qqks5uON_RgaJpZM4ShF_6
.

@ChrisChiasson, I probably should have asked this earlier on. It just dawned on me.
In your experimentation, did you ever write a template specialization for any of these?

std::complex<stan::math::var>
std::complex<stan::math::fvar<double>>
std::complex<stan::math::fvar<stan::math::var>>
std::complex<stan::math::fvar<stan::math::fvar<double>>>
std::complex<stan::math::fvar<stan::math::fvar<stan::math::var>>>

Would it be possible to write an explicit template specialization for just the constructor? Here's a short example along the lines of what I'm thinking (but I haven't done it for complex):
#include <stan/math.hpp>
#include <iostream>

template <typename T = double>
class foo {
 public:
  foo(T x = T(), T y = T())
      : x_(x), y_(y) {
  }
  std::string print() {
    std::stringstream m;
    m << "(" << x_ << ", " << y_ << ")" << std::endl;
    return m.str();
  }
  T x_, y_;
};

template<> foo<stan::math::var>::foo(stan::math::var x, stan::math::var y) {
  if (is_uninitialized(x))
    x_ = 0.0;
  if (is_uninitialized(y))
    y_ = 0.0;
}

int main() {
  foo<double> d;
  foo<stan::math::var> v;
  std::cout << "d = " << d.print() << std::endl;
  std::cout << "v = " << v.print() << std::endl;
  return 0;
}

If we could just write a constructor for each, that would eliminate the need for zeroing, the template metaprograms (e.g. rm_zeroing), and simplify a bunch of it? I'm not sure if you've already explored this and found that it wasn't possible.

The problem occurrs inside some of the algorithms. The class type is
already T. The algorithms call T(). If T is var, go directly to jail, do
not collect $200. It is unfortunate, but T isn't std::complex<var>. BTW,
your solution would change fvar<var> ... not just for the complex class.
I did at one time post an outline of how to fix var itself, by modifying
the struct that contains chainable so that it maintained a vector stack of
zeros whose addresses would be usedwith default constructed vars. However I
was asked not to pursue it.
On Mon, Aug 6, 2018, 7:57 PM Daniel Lee<EMAIL_ADDRESS>wrote:

@ChrisChiasson https://github.com/ChrisChiasson, I probably should have
asked this earlier on. It just dawned on me.
In your experimentation, did you ever write a template specialization for
any of these?

std::complexstan::math::var
std::complex<stan::math::fvar>
std::complex<stan::math::fvarstan::math::var>
std::complex<stan::math::fvar<stan::math::fvar>>
std::complex<stan::math::fvar<stan::math::fvarstan::math::var>>

Would it be possible to write an explicit template specialization for just
the constructor? Here's a short example along the lines of what I'm
thinking (but I haven't done it for complex):
#include <stan/math.hpp>
#include
template
class foo {
public:
foo(T x = T(), T y = T())
: x_(x), y_(y) {
}
std::string print() {
std::stringstream m;
m << "(" << x_ << ", " << y_ << ")" << std::endl;
return m.str();
}
T x_, y_;
};
template<> foostan::math::var::foo(stan::math::var x, stan::math::var y) {
if (is_uninitialized(x))
x_ = 0.0;
if (is_uninitialized(y))
y_ = 0.0;
}
int main() {
foo d;
foostan::math::var v;
std::cout << "d = " << d.print() << std::endl;
std::cout << "v = " << v.print() << std::endl;
return 0;
}
If we could just write a constructor for each, that would eliminate the
need for zeroing, the template metaprograms (e.g. rm_zeroing), and
simplify a bunch of it? I'm not sure if you've already explored this and
found that it wasn't possible.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-410898478, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALTv-6LaaumX7IrcpGJNiae_LdNUfRvaks5uOOYHgaJpZM4ShF_6
.

Can you be more specific? I have no idea what you mean by "some of the
algorithms." If you have any more information than that, that'd be
enlightening. I just thought you were having to go through this effort
because of the constructor to std::complex.
On Mon, Aug 6, 2018 at 9:12 PM ChrisChiasson<EMAIL_ADDRESS>wrote:

The problem occurrs inside some of the algorithms. The class type is
already T. The algorithms call T(). If T is var, go directly to jail, do
not collect $200. It is unfortunate, but T isn't std::complex<var>. BTW,
your solution would change fvar<var> ... not just for the complex class.
I did at one time post an outline of how to fix var itself, by modifying
the struct that contains chainable so that it maintained a vector stack of
zeros whose addresses would be usedwith default constructed vars. However I
was asked not to pursue it.
On Mon, Aug 6, 2018, 7:57 PM Daniel Lee<EMAIL_ADDRESS>wrote:

@ChrisChiasson https://github.com/ChrisChiasson, I probably should
have
asked this earlier on. It just dawned on me.
In your experimentation, did you ever write a template specialization for
any of these?

std::complexstan::math::var
std::complex<stan::math::fvar>
std::complex<stan::math::fvarstan::math::var>
std::complex<stan::math::fvar<stan::math::fvar>>
std::complex<stan::math::fvar<stan::math::fvarstan::math::var>>

Would it be possible to write an explicit template specialization for
just
the constructor? Here's a short example along the lines of what I'm
thinking (but I haven't done it for complex):
#include <stan/math.hpp>
#include
template
class foo {
public:
foo(T x = T(), T y = T())
: x_(x), y_(y) {
}
std::string print() {
std::stringstream m;
m << "(" << x_ << ", " << y_ << ")" << std::endl;
return m.str();
}
T x_, y_;
};
template<> foostan::math::var::foo(stan::math::var x, stan::math::var
y) {
if (is_uninitialized(x))
x_ = 0.0;
if (is_uninitialized(y))
y_ = 0.0;
}
int main() {
foo d;
foostan::math::var v;
std::cout << "d = " << d.print() << std::endl;
std::cout << "v = " << v.print() << std::endl;
return 0;
}
If we could just write a constructor for each, that would eliminate the
need for zeroing, the template metaprograms (e.g. rm_zeroing), and
simplify a bunch of it? I'm not sure if you've already explored this and
found that it wasn't possible.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-410898478, or
mute
the thread
<
https://github.com/notifications/unsubscribe-auth/ALTv-6LaaumX7IrcpGJNiae_LdNUfRvaks5uOOYHgaJpZM4ShF_6
.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-410900544, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAZ_F5CzogMjpXy1OZF7P-H3Sn7EVGqAks5uOOldgaJpZM4ShF_6
.

Try searching this file for _Tp(
https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/std/complex
On Mon, Aug 6, 2018, 8:16 PM Daniel Lee<EMAIL_ADDRESS>wrote:

Can you be more specific? I have no idea what you mean by "some of the
algorithms." If you have any more information than that, that'd be
enlightening. I just thought you were having to go through this effort
because of the constructor to std::complex.
On Mon, Aug 6, 2018 at 9:12 PM ChrisChiasson<EMAIL_ADDRESS>wrote:

The problem occurrs inside some of the algorithms. The class type is
already T. The algorithms call T(). If T is var, go directly to jail, do
not collect $200. It is unfortunate, but T isn't std::complex<var>.
BTW,
your solution would change fvar<var> ... not just for the complex
class.
I did at one time post an outline of how to fix var itself, by modifying
the struct that contains chainable so that it maintained a vector stack
of
zeros whose addresses would be usedwith default constructed vars.
However I
was asked not to pursue it.
On Mon, Aug 6, 2018, 7:57 PM Daniel Lee<EMAIL_ADDRESS>wrote:

@ChrisChiasson https://github.com/ChrisChiasson, I probably should
have
asked this earlier on. It just dawned on me.
In your experimentation, did you ever write a template specialization
for
any of these?

std::complexstan::math::var
std::complex<stan::math::fvar>
std::complex<stan::math::fvarstan::math::var>
std::complex<stan::math::fvar<stan::math::fvar>>
std::complex<stan::math::fvar<stan::math::fvarstan::math::var>>

Would it be possible to write an explicit template specialization for
just
the constructor? Here's a short example along the lines of what I'm
thinking (but I haven't done it for complex):
#include <stan/math.hpp>
#include
template
class foo {
public:
foo(T x = T(), T y = T())
: x_(x), y_(y) {
}
std::string print() {
std::stringstream m;
m << "(" << x_ << ", " << y_ << ")" << std::endl;
return m.str();
}
T x_, y_;
};
template<> foostan::math::var::foo(stan::math::var x, stan::math::var
y) {
if (is_uninitialized(x))
x_ = 0.0;
if (is_uninitialized(y))
y_ = 0.0;
}
int main() {
foo d;
foostan::math::var v;
std::cout << "d = " << d.print() << std::endl;
std::cout << "v = " << v.print() << std::endl;
return 0;
}
If we could just write a constructor for each, that would eliminate the
need for zeroing, the template metaprograms (e.g. rm_zeroing), and
simplify a bunch of it? I'm not sure if you've already explored this
and
found that it wasn't possible.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-410898478, or
mute
the thread
<

https://github.com/notifications/unsubscribe-auth/ALTv-6LaaumX7IrcpGJNiae_LdNUfRvaks5uOOYHgaJpZM4ShF_6

.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-410900544, or
mute
the thread
<
https://github.com/notifications/unsubscribe-auth/AAZ_F5CzogMjpXy1OZF7P-H3Sn7EVGqAks5uOOldgaJpZM4ShF_6
.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-410901228, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALTv-6GHjxTnMwLNDZHijS5aM31lxe5eks5uOOp9gaJpZM4ShF_6
.

Just to take inventory:

the default constructor (which we can template specialize): https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/std/complex#L129
operator= for assigning a real to a complex (which we should be able to specialize?): https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/std/complex#L235
operator== for complex, real. Isn't this ok? https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/std/complex#L463
operator!= for the same reasons: https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/std/complex#L481
__complex_abs. I don't think there's anything wrong with this implementation. https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/std/complex#L593

There are another half dozen places where it pops up. I don't see a problem with them, do you?

@ChrisChiasson, I searched through your tests. What I didn't find were basic tests for std::complex<stan::math::var> and all the other types. Can you either point me to them or can you create the tests for them?
This would be a requirement for accepting this sort of code. What I mean by tests:

test that you can instantiate std::complex<T> where T is {stan::math::var, stan::math::fvar<double>, stan::math::fvar<stan::math::var>}. You could do fvar<fvar<var>>, but I think that'd cover it.
test for each of the operators: (*, /, +, =, !=, *=, +=, etc.); it looks like we should be testing with complex arguments and real arguments (based on what you pointed out with the_Tp())
if you're so inclined, you should test the few functions that have the _Tp() in the implementation.

Why here and not in something like std::vector<T>? It's because std::vector<T> is specified for custom types and we obey all the constructs. Here, the implementation may change and the only thing we have to know that it's still good is if we have tests.
I have a feeling that once we create these tests, we can replace the implementation with something a lot simpler: getting rid of zeroing, rm_zeroing, etc. And then we can merge easily.

I respectfully decline.Thank you for your time. Take care, all.

Thanks for understanding. Am I ok to assume the code that was written is
considered BSD (with the copyright owners appropriately attributed)?
Hopefully someone will want to finish this PR out by writing the basic
tests required for it to get into the Math library.
Sorry about the length of time it took to get to this point. Would you mind
jumping on a video call with me to talk a little bit about the process? I’d
want to know how we could have provided better guidelines earlier or
communicated better to set expectations appropriately.
Thanks for putting in all this work. You’ve figured out a ton of
implementation details I wouldn’t have known how to figure out. I’ll try to
write out as much detail I can remember in the issue, but if there’s
anything I missed, would you mind helping update it? Thanks again.
On Wed, Aug 8, 2018 at 3:12 PM ChrisChiasson<EMAIL_ADDRESS>wrote:

I respectfully decline.Thank you for your time. Take care, all.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/stan-dev/math/pull/789#issuecomment-411519385, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAZ_F1VgahkgZOgOTru5HYCS3W-osCt7ks5uOzf7gaJpZM4ShF_6
.


Remove from ABI, but keep in API

This one is going to be controversial... but isn't it beautiful :innocent: ?
Even if it is ultimately rejected, I believe it is worth to put into condideration.
Implemented so far:

Remove most deprecated types/constants/functions.
Add backward API compatibility #define's for the removals.
Add backward API compatibility static inline functions.

For further consideration:

MPI_Info_get[_valuelen]() are implemented with static inline functions. inline is not in C89, but in that case most compilers support the __inline keyword as an extension.
Maybe we could do #define MPI_T_ERR_INVALID_ITEM MPI_T_ERR_INVALID_ITEM to get rid of the deprecated enum value?
What to do with MPI_HOST? Maybe #define MPI_HOST MPI_KEYVAL_INVALID to defer failure to runtime?

Maybe we could do #define MPI_T_ERR_INVALID_ITEM MPI_T_ERR_INVALID_ITEM to get rid of the deprecated enum value?

What?

Maybe we could do #define MPI_T_ERR_INVALID_ITEM MPI_T_ERR_INVALID_ITEM to get rid of the deprecated enum value?

What?

Sorry, bad copy & paste, I meant
#define MPI_T_ERR_INVALID_ITEM MPI_T_ERR_INVALID_INDEX


Copilot not using references, happens a lot, it's getting annoying

I apologize for the confusion. The reference you're seeing is a simulated concept for the purpose of this scenario. It's meant to represent the idea that you've provided a file or code snippet for review. However, as an AI, I don't have the ability to access or view files. I can only provide assistance based on the information you provide in the text of your messages.

I'm sorry for any confusion, but as an AI, I don't have the ability to see what you're highlighting or access any files on your computer. I can only provide assistance based on the information you provide in the text of your messages. If you could copy and paste the relevant parts of the PromptServer class into a message, I would be able to provide more specific advice on how to modify it to use HTTPS.

did they even test this for one second.  the broke the usefullness

It's not clear what your scenario is. Maybe this is related to the change described in https://code.visualstudio.com/updates/v1_86#_default-context-in-chat. But maybe you can explain some more

It's not clear what your scenario is. Maybe this is related to the change described in https://code.visualstudio.com/updates/v1_86#_default-context-in-chat. But maybe you can explain some more

Thanks, that is working for me.

I think this will certainly help. The issue seemed to pop up whenever I had used a statement like "in my code" "my reference", but even if I stopped using terminology as such, it would get stuck on the idea for a while. I think this can be closed for now. I just have to be carefull not to use certain words and use the default context in chat opperators.


fixed linting errors, moved logout function to dashboard header, changed app.…

…config.js back to app.json and updated babel.config.js to utilize react native dotenv with firebase config
Description
troubleshooting!  fixed linting errors in errandtracker files. changed app.config.js back to app.json and updated babel.config.js to enable utilizing react native dotenv in firebase config files, moved firebase signout function to dashboard header.
How to test
load app in simulator. login in with email and password saved in firebase or create new account by clicking register and filling out form then clicking 'create account.' user will be taken to dashboard. clicking on errand tracker will take user to errand tracker. clicking on 'sign out' in dashboard header will logout user and take them back to login screen.
Notes
you must have api keys for google and firebase.
Issue tracking
(https://trello.com/c/efC18pdg/42-fix-linting-and-env-processing-errors)

 fix linting and .env processing errors


Can not receive job status change message

Organization Name:

Short summary about the issue/question:
recently, I can not receive job status change message, the error log of alter-manager is like below:
2022-10-25T13:18:52.018Z [ERROR] Failed when handle job status change for job test-code-server_18304946:
meta = {
"message": "read ECONNRESET",
"stack": "Error: read ECONNRESET\n    at TCP.onStreamRead (internal/stream_base_commons.js:111:27)",
"config": {
"url": "http://<IP_ADDRESS>:80/alert-manager/api/v1/alerts",
"method": "post",
"data": "[]",
"headers": {
"Accept": "application/json, text/plain, /",
"Content-Type": "application/json",
"User-Agent": "axios/0.21.1",
"Content-Length": 2
},
"transformRequest": [
null
],
"transformResponse": [
null
],
"timeout": 0,
"xsrfCookieName": "XSRF-TOKEN",
"xsrfHeaderName": "X-XSRF-TOKEN",
"maxContentLength": -1,
"maxBodyLength": -1
},
"code": "ECONNRESET"
}
Brief what process you are following:

How to reproduce it:

OpenPAI Environment:

OpenPAI version:
Cloud provider or hardware configuration:
OS (e.g. from /etc/os-release):
Kernel (e.g. uname -a):
Hardware (e.g. core number, memory size, storage size, GPU type etc.):
Others:

Anything else we need to know:

fix it by modify job status notification source code to skip empty alters


Update docs for v0.7

Staged at:

README
User guide

/assign @yiqigao217
/assign @GinnyJI
/cc @rjbez17

I think it's fine to remove this hold, we can always clean it up tomorrow :)
/hold cancel
On Mon, Dec 14, 2020 at 5:39 PM Yiqi Gao<EMAIL_ADDRESS>wrote:

@yiqigao217 commented on this pull request.
/lgtm
/hold
Just for another eye. Feel free to remove the hold anyone :)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes-sigs/multi-tenancy/pull/1322#pullrequestreview-551953711,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AE43PZB4QIRE4GDFYCHGWDDSU2H2FANCNFSM4U3NMRMQ
.


Support component hooks closures

Objective

Adopted #12287

Solution

Updated the Component macro to accept closures for ComponentHooks.

Testing

Compiled locally and tested by altering the component_hooks example.

I still have some outstanding questions regarding this change:

[ ] Is the example update any good? Or should I consider doing a "full rework" where we actually showcase the three ways of adding hooks to components in the actual code?
[ ] Should we consolidate RequiredFunc and HookFunc enums since they are essentially the same? I went for a simpler approach but was thinking on creating some DeriveFunc (or similar) to cover both cases and avoid code repetition.

shouldn't this have the changes from bevy_ecs/src/components.rs in #12287 ?
Looking at the discussion in that pr, I'd vote not to support closures for hooks unless a user requests it. Fn can only borrow immutably, which doesn't feel like it adds much functionality here so I'm not sure how useful it would be.


Detect non-virtual environments from Add virtual env dialog

If you go through the Add virtual env dialog and select a non-virtual environment, we will refuse to add it - the correct path is to add a custom global environment through the Python Environments window. However, we don't make this at all obvious.
Similarly, if you go to Add/Remove Environments then we don't help you add a custom one (there's a link to go online and install one, but that's it).
In both of these dialogs, we should have a way to redirect users to the Python Environments window and get them into the Add Custom Environment view.

Add existing virtual env and add existing custom env are now both done in the same UI.


PathLike wrapper/cache for ExternalStorage

This is a start to solving #120. We've identified the following goals:

We want to be able to use an arbitrary external backend to shared (as well as permanent), which we already have via ExternalStorage, instead of it needing to be a directory on the same host.
We want the user experience to mirror conveniences of pathlib.Path, which will (we hope) minimize frustration with the transition for users.

So far, this PR doesn't integrate that into any of the execution process. That can be either part of this PR or a future one.
The basic approach here involves 2 classes:

SharedRoot: represents the root directory for a given unit. By default, this will exist at $SCRATCH/.holding/$UNIT_LABEL. This is where most of the actual logic is. It contains references to all the paths that have been registered with it. On deletion (or on demand) this transfers all paths registered with it to the ExternalStorage.
SharedPath: represents any path within SharedRoot. Contains information on the actual file path on the system, as well as the label used as the key for the external key-value store. Creation is typically from root / "some_path" or some_path / "subpath". A SharedPath can be used like any other PathLike in open, and can be converted via pathlib.Path(shared_path) to get all the functionality of pathlib.

As I'm opening this, code is still in progress. But at least the tests should give a picture of usage; please take a look!

Still in progress, but here's a quick update to what is included in this PR now. Before I dive any further into writing unit tests, take a quick look, @richardjgowers (at least at the contents of this comment).
Main idea: We have "staging" directories that are local. From a protocol author's perspective, shared, permanent, and scratch are PathLikes. However, the paths pointed to by shared and permanent are in the local scratch space, and the files there are transferred to external storage as part of the executor.
I've expanded this PR to include some tools for handling the entirety of the scratch / shared / permanent storage lifecycle. There are two context managers: one for the unit level that provides the PathLikes that are used by the protocol author, and which handles the cleanup at the end of that level, and one for the dag level that provides the thing that creates the unit-level context manager, and which cleans up at the dag level.
Here's an example from a protocol author's viewpoint (kind of faking our real units, but this gives the rough picture):
https://github.com/OpenFreeEnergy/gufe/blob/ed5e83c23e3ee03534f8cbe5af753cc4ac9baeae/gufe/tests/storage/test_storagemanager.py#L27-L53
For the executor, the basic picture is something like this:
manager = StorageManager(scratch, shared_external, permanent_external)
with manager.running_dag(dag_label) as dag_ctx:
    for unit in ordered_dag_units:
        with dag_ctx.running_unit(unit.key) as (scratch, shared, permanent):
            unit_result = unit.run(scratch, shared, permanent)  # or however we run the unit
        # when this context ends, scratch will be deleted, shared external storage will be
        # updated, and shared holding will be deleted
# when this context ends, permanent external storage will be updated, and permanent holding
# will be deleted

For much more detail, see this lifecycle test:
https://github.com/OpenFreeEnergy/gufe/blob/ed5e83c23e3ee03534f8cbe5af753cc4ac9baeae/gufe/tests/storage/test_storagemanager.py#L59-L131
Next tests will be to check the special cases that shared or permanent represent the same directories as the staging directory, or each other.

From power hour:
         with shared.other_shared("unit1") as prev_shared:
             with open(prev_shared / "bar.txt", mode='r') as f:
                 bar = f.read()

Instead of having shared.other_shared("unit1"), which would allow a e.g. unitX to access unitA's shared space even if unitA wasn't an explicit dependency of unitX.
Instead, making unitX access unitA's shared via its ProtocolDAGResult object passed into run would avoid this issue.

@richardjgowers @dotsdl : This is pretty much ready for an initial review pass. There are a few lines that need coverage, but most of them are edge cases where I thought "not bad enough to raise an exception, but maybe I should log it," so I need to create those edge cases and ensure that the expected message gets logged.
New directory structure this uses:
local_root
├── scratch
│   └── dag
│       └── unit2
└── .staging
    └── dag
        ├── unit1
        └── unit2

Note that the name .staging is customizable; the executor can set it to "", in which case you have:
local_root
├── dag
│   ├── unit1
│   └── unit2
└── scratch
    └── dag
        └── unit2

In this way, the "staging" files can be final output location for local runs. (This is what I'd like to do with quickrun.) That way, if someone wants to check on the initial units of a DAG, the location is the same as checking those units after the DAG has completed (thinking of the people who just want to work with the filesystem).

Hello @dwhswenson! Thanks for updating this PR.


Escape from Tarkov (DX11 Unity 5 game) crashing on map load when using more than declared RAM

That's quite odd issue that I've been trying to troubleshoot for the previous couple of days. Mind me being beginner, so it may be something obvious I'm missing.
Escape from Tarkov runs generally without issues on DXVK, but I am trying to see if it'll run on 4 GB for the friend of mine. The PC I tested it have 16 GB of RAM (2x 8 GB), so I used the mem=4G kernel parameter to limit my RAM, and gave Linux 8 GB swap partition. The game generally runs fine in menu and in the small "Factory" map, but when loading bigger "Customs" map, the game starts to use more than 4 GB of RAM (it's 64-bit executable), swap is slowly being filled when the map load, but the Unity crashes with the attached error when the map finishes loading, timer goes to "0" and actual gameplay tries to load. It only happens with DXVK – the game loads on Windows and Wine's D3D11 using swap, only stutter is more noticable obviously because of the swap being used instead of RAM.
I tried all kind of settings, including setting vm.swappiness=100, zswap.enabled=1 zswap.compressor=zstd zswap.max_pool_percent=100 zswap.zpool=z3fold kernel parameters for zswap, I run the game on separate X server, so there's nothing running in the background except st htop (for monitoring), xorg, systemd and the game itself (running on ALSA), I've read through entirety of #747 and tried all the tweaks, including:

Modify /etc/tmpfiles.d/local.conf to disable Transparent Huge Pages entirely:

w /sys/kernel/mm/transparent_hugepage/enabled - - - - never
w /sys/kernel/mm/transparent_hugepage/defrag - - - - never

Fiddling with dxvk.conf, including

dxgi.maxDeviceMemory = 1024
dxgi.maxSharedMemory = 1024

to limit GPUs memory below 4 GB (this PC runs NVIDIA's GeForce GTX 960 with 4 GB VRAM) and/or
dxvk.allowMemoryOvercommit = True

to allow using more memory, and various other dxvk.conf variables. I also tried DXVK_STATE_CACHE=0, but it didn't help.
I use the tkg's PKGBUILD with DXVK embedded, running the latest master of everything, and I'm now in the process of building nvidia-all from it to see if it changes anything. I didn't try using custom kernels, but I doubt it will change anything (I'm running stock 4.20.3.arch1-1 kernel from Arch's repos).
I can't test whether it's the mem=4G kernel parameter that's causing issues and whether the game would ran fine with regular 4 GB of RAM, as I don't own a 4 GB DDR3 stick.
It's also worth noting that the game also runs fine on my 16 GB RAM with AMD's Radeon RX480 with 8 GB VRAM rig, but limiting RAM to 4GB hangs my CPU as I don't have swap at all.
Is there anything I can do to make it use swap and prevent it crashing with 4 GB? Will it crash on physical 4 GB too and I am forced to run this particular map on Wine's D3D11 renderer? Thanks in advance for any pointers :). I'll let you know if I find something new too, and will try to get my hand on a working 4 GB RAM stick to hopefully test it proper.
Software information
Escape from Tarkov <IP_ADDRESS>4 (Unity 5.6.6f2, D3D11 renderer), 1024x768, all settings to lowest. The game have "ram cleaning" option, I tried it both on and off. I can also post content of my game settings files if needed.
System information

GPU: NVIDIA GeForce 960 GTX 4 GB
Driver: 415.25.0
Wine version: 4.0-rc7 (Staging Esync PBA Dxvk) – from Tkg's PKGBUILD
DXVK version: v0.95-17-g4d4bd182

Apitrace file(s)
Since it's not crashing with regular D3D11 I didn't do apitrace, as from what I undestand I have to do that without DXVK, but I can do so if instructed (I never done it, but guess I'll manage to do so if needed).
Log files

d3d11.log: https://gist.githubusercontent.com/Faalagorn/89e0de3e4a1e547f39a745523dd78b32/raw/93765aefd8e6eda5a3787bc51dcb068deaaa59a3/EscapeFromTarkov_d3d11.log
dxgi.log: https://gist.githubusercontent.com/Faalagorn/89e0de3e4a1e547f39a745523dd78b32/raw/93765aefd8e6eda5a3787bc51dcb068deaaa59a3/EscapeFromTarkov_dxgi.log
I can also attach Unity's crash files (output_log.txt, error.log and crash.dmp)

Since the game launches with dedicated launcher, there's also BsgLauncher_d3d11.log, BsgLauncher_dxgi.log, CefSharp.BrowserSubprocess_d3d11.log and CefSharp.BrowserSubprocess_dxgi.log, but the launches closes as soon as the game's launched, so it shouldn't matter, but I can attach them if needed.
I generally launch with export WINEDEBUG=-all especially since when I tried to launch it with reverse export WINEDEBUG=+all the game didn't want to run. Wine generally does not crashed too, but it did once and allowed me to save the backtrace.txt, so I can upload it too.

I had the same issue, till i turned on "Auto Ram Cleaner" in the Game Option
DXVK Version: 1.0.3
WINE Version: tkg-4.2-x86_64
Installed with Lutris (Runner from lutris.net) - not it works fine!

@montify Hm, so it does seem to be related to RAM after all I guess? Out of curiosity, how much system RAM do you have total? Is it 4GB or more?

Yes, a Ram problem, but i dont know if its a dxvk or Wine issue
i have 10GB Ram

So i can confirm this erorr occurs when the gpu run out of vram.
i close the Webbrowser(consume ~300mb VRAM whit some tabs open) und now the problem is gone.
@doitsujin
How DXVK Handle this when the gpu run out of vram?

When you run out of VRAM DXVK allocates the resources in RAM. If Escape from Tarkov is a 32 Bit game, it may run out of address space. That's a known problem with DXVK and pretty much impossible to fix.

Yeah, that. With Proton and certain Wine builds (-tkg, not sure if it's in staging), you can also set WINE_LARGE_ADDRESS_AWARE=1 to double the amount of virtual memory, and DXVK 1.1 (aka latest -git) also has some code to reduce RAM usage especially in games with a large number of shaders, but 32-bit games usually don't suffer from that problem.

Tarkov is a 64bit Application, so this is not the issue i guess..
I asking myself if it is a DXVK, Vulkan or Wine issue?!

Yup, Escape from Tarkov it's a pure 64-bit application, including the laucher. It runs even with wine version compiled without 32-bit support (I run it like that myself) so I doubt WINE_LARGE_ADDRESS_AWARE will help, but I didn't test it since the original report, so maybe I'll see how it is with latest DXVK.

Using DXVK 1.3 produces this error on crash
https://gist.github.com/maverck/4aef4dbb7b394cf3487343818eba2824

That's the Nvidia allocation failure again. See the issue referenced above your comment for more information.

I'll close this for now, observe #1100 for potential fix on the memory fix!
In the meantime, I'll try to see if the unrelated crash that's been reported in https://forum.escapefromtarkov.com/topic/1717-linux-support/ (10-ish+ pages)


try string instead...

Signed-off-by: Ozzy Osborne<EMAIL_ADDRESS>

This change is

Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable


Project deletion currently not working?

When I press the delete button on a project, I get this in the console:
Acess to fetch at 'http://localhost:8011/api/v1/project/1' from origin 'http://localhost:5173' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. If an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled.

http://localhost:8011/api/v1/project/1 net::ERR_FAILED 500 (Internal Server Error)
window.fetch @ fetcher.js?v=53bac609:54
deleteProject @ project_api.js:87
handleDeleteProject @ ProjectsList.svelte:42
handleCallbackAction @ ConfirmActionButton.svelte:14
ProjectsList.svelte:47 TypeError: Failed to fetch
    at window.fetch (fetcher.js?v=53bac609:54:10)
    at deleteProject (project_api.js:87:26)
    at handleDeleteProject (ProjectsList.svelte:42:11)
    at HTMLButtonElement.handleCallbackAction (ConfirmActionButton.svelte:14:38)
(

I see nothing on the Svelte server or Fractal server though.
Regarding the CORS: All other calls (creating projects, new datasets, submitting workflows etc.) appears to be working, so not sure why this call fails.
I'm running fractal server 1.2.4, current main fractal web.
Deployment-wise, I'm using a postgres backend, gunicorn and an httpd proxy server in front of fractal-web
(on sqlite, project deletion leads to database issues on the fractal server side)

Actually, I think it's a server-side issue. Even trying to delete from the command line leads to a 500 server error. The server logs still don't show anything the default logging setup, but deletion fails:
fractal project delete 1
ERROR:root:Server returned 500
ERROR:root:Original request: DELETE http://localhost:8011/api/v1/project/1
ERROR:root:Original payload:
ERROR:root:Server error message: {}

ERROR:root:Terminating.


isFocused not updating after first navigation

I've tried both approaches and end up with the same result:
// 1.
const { isFocused } = useNavigationFocus()

// 2.
const navigation = useNavigation();
const isFocused = navigation.isFocused();

This is in the context of a Tab Navigator. The first time I switch tab isFocused is updated - but after the initial render it doesn't update.
For now I went back to use withNavigationFocus- I get a feeling this package needs some more testing before using! :)

Hi,
I can confirm, it's an existing bug in the event hook which affect focus as well.
Made this PR long time ago to fix this: https://github.com/react-navigation/hooks/pull/8
I'd like to get this fixed soon, but need to improve the lib's DX first. There's no tests and no example app. I've a pending PR which does not work yet for the example app: https://github.com/react-navigation/hooks/pull/28

I just did a direct implementation that I think is stable that doesn't depend on anything but the useNavigation() hook, maybe it can be of help:
import { useState, useEffect } from 'react';
import { useNavigation } from 'react-navigation-hooks';

const useIsFocused = () => {
  const navigation = useNavigation(),
        [isFocused, setFocused] = useState(navigation.isFocused());

  useEffect(() => {
    const didBlur = () => setFocused(false);
    const didFocus = () => setFocused(true);

    const blurSubscription = navigation.addListener('didBlur', didBlur);
    const focusSubscription = navigation.addListener('didFocus', didFocus);

    return () => {
      blurSubscription.remove();
      focusSubscription.remove();
    };
  }, []); //eslint-disable-line

  return isFocused;
};

export default useIsFocused;

@robertherber what is useNavigationFocus() ?
Actually I'm looking at the code and don't see it. There is "useFocusState().isFocused" and according to my tests and the code I'm reading, it's working.
Is useNavigationFocus() a custom hook you built on events? I suspect you have a custom hook, and you initialize focus state to false, and the initial focus event is not fired on mount (which I'm currently correcting)

Will be fixed by https://github.com/react-navigation/hooks/pull/38, going to merge/release soon

should be fixed by release 1.0.3 and #38, please tell me if everything works


if polar is selected the IRamanWorkChain should take that into account to run the average workchain

As pointed out by @AndresOrtegaGuerrero .

@mikibonacci I think this might not be needed (we need to double check), since the widget of visualisation can compute already the power and.or the crystal spectrum. We can double check with Lorenzo , but i think with the widget the user can actually decide which spectrum they want.

Lorenzo told me that the average computed in the average workchain is different: it integrates on q. instead, the direct function to compute powder just an average on the irreducible modes. However, I will open another issue on this, as in the new aiida-vibroscopy release there will be a simple function (not aiida) to do also this.

i checked his workchain, https://github.com/bastonero/aiida-vibroscopy/blob/main/src/aiida_vibroscopy/workflows/spectra/intensities_average.py , what is the difference ?

I checked more deeply, and actually I think nothing, apart the fact that to have the q integral we just have to define the quadrature order. So if we define it in the app, should work.

ahh ok !  then we have to modify the workchain indeed


Fix condition in exactPackageName function

Description of the PR
Fixes #1565
Corrects the condition in exactPackageName such that not providing a package namespace will result in actually exiting the function early as intended. exactPackageName is intended to only continue if each of the package type, namespace, and name are present.

PR Checklist

[ ] All commits have a Developer Certificate of Origin (DCO) -- they are generated using -s flag to git commit.
[ ] All new changes are covered by tests
[ ] If GraphQL schema is changed, make generate has been run
[ ] If collectsub protobuf has been changed, make proto has been run
[ ] All CI checks are passing (tests and formatting)
[ ] All dependent PRs have already been merged

Thanks for the fix. Looks like there might be more going on, as the e2e reports some issue.
The query (https://github.com/guacsec/guac/blob/main/demo/graphql/queries.gql#L78-L85) is not returning the expected value (https://github.com/guacsec/guac/blob/main/internal/testing/e2e/expectIsDependencyQ2.json)
@@@@ Running queries and validating output
--- /home/runner/work/guac/guac/internal/testing/e2e/expectIsDependencyQ2.json	2023-12-27 18:03:45.165486707 +0000
+++ /home/runner/work/guac/guac/gotIsDependencyQ2.json	2023-12-27 18:06:19.445611726 +0000
@@ -1,54 +1,3 @@
 {
-  "IsDependency": [
-    {
-      "justification": "top-level package GUAC heuristic connecting to each file/package",
-      "versionRange": "v1.4.2",
-      "package": {
-        "type": "guac",
-        "namespaces": [
-          {
-            "namespace": "cdx/docker.io/library",
-            "names": [
-              {
-                "name": "consul",
-                "versions": [
-                  {
-                    "version": "sha256:22ab19cf1326abbfaafec6a14eb68f96e899e88ffe9ce26fa36affcf8ffb582c",
-                    "qualifiers": [
-                      {
-                        "key": "tag",
-                        "value": "latest"
-                      }
-                    ],
-                    "subpath": ""
-                  }
-                ]
-              }
-            ]
-          }
-        ]
-      },
-      "dependencyPackage": {
-        "type": "golang",
-        "namespaces": [
-          {
-            "namespace": "github.com/sirupsen",
-            "names": [
-              {
-                "name": "logrus",
-                "versions": [
-                  {
-                    "version": "v1.4.2",
-                    "qualifiers": [],
-                    "subpath": ""
-                  }
-                ]
-              }
-            ]
-          }
-        ]
-      },
-      "collector": "FileCollector"
-    }
-  ]
+  "IsDependency": []
 }

I'm wondering if name-namespace-name are given that it will only try to match against the package-name node, and not check the matching package-version nodes as well.

Fixed by #1627


[yaml] Normalize RunInference transform

What would you like to happen?
This bug is part of the Beam YAML milestone. More information about Beam YAML can be found here: https://github.com/apache/beam/tree/master/sdks/python/apache_beam/yaml#beam-yaml-api
A Beam YAML contribution guide can be found here: https://s.apache.org/beam-yaml-contribute

The RunInference transform should be normalized to be compatible with Beam YAML. There is a doc for normalizing the Beam IO's for Beam YAML, but the general process can be carried over to this work. That doc can be found here.
Issue Priority
Priority: 3 (nice-to-have improvement)
Issue Components

[X] Component: Python SDK
[ ] Component: Java SDK
[ ] Component: Go SDK
[ ] Component: Typescript SDK
[ ] Component: IO connector
[X] Component: Beam YAML
[ ] Component: Beam examples
[ ] Component: Beam playground
[ ] Component: Beam katas
[ ] Component: Website
[ ] Component: Spark Runner
[ ] Component: Flink Runner
[ ] Component: Samza Runner
[ ] Component: Twister2 Runner
[ ] Component: Hazelcast Jet Runner
[ ] Component: Google Cloud Dataflow Runner

.take-issue

@Polber can you guide me here

I would suggest taking a look at one of the other transforms that need normalizing:
https://github.com/apache/beam/issues?q=is%3Aopen+is%3Aissue+label%3Ayaml+normalize
TFRecord would be a good candidate if your interest is in ML transforms