Putting the quick in quick tree: compile an instantiation function for each class model type

I was curious about how hard and how effective the "compile a function that does the same thing but with loops unrolled and indirection removed" approach would be for model instantiation. I tried it. It works ... really well.
On main:
❯ p x bench/create-large-root.ts

><EMAIL_ADDRESS>x /Users/airhorns/Code/mobx-quick-tree
> ts-node --transpile-only "bench/create-large-root.ts"

instantiating a large root x 395 ops/sec ±0.88% (90 runs sampled)

On the first pass optimization branch in #44:
❯ p x bench/create-large-root.ts

><EMAIL_ADDRESS>x /Users/airhorns/Code/mobx-quick-tree
> ts-node --transpile-only "bench/create-large-root.ts"

instantiating a large root x 925 ops/sec ±1.01% (88 runs sampled)

On this branch:
❯ p x bench/create-large-root.ts

><EMAIL_ADDRESS>x /Users/airhorns/Code/mobx-quick-tree
> ts-node --transpile-only "bench/create-large-root.ts"

instantiating a large root x 3,182 ops/sec ±0.26% (97 runs sampled)

I am surprised and delighted 💨 💨 💨 . I can also confirm this is real, I pushed a prerelease to Gadget to see if I was smoking performance-enhancing drugs and https://github.com/gadget-inc/gadget/pull/6444 is green!
The old approach that instantiated every class model looked like this:
export const instantiateInstanceFromProperties = (
  instance: any,
  snapshot: Record<string, any> | undefined,
  properties: ModelProperties,
  identifierProp: string | undefined,
  context: InstantiateContext
) => {
  for (const propName in properties) {
    const propType = properties[propName];
    if (isReferenceType(propType.mstType)) {
      context.referencesToResolve.push(() => {
        const propValue = propType.instantiate(snapshot?.[propName], context);
        instance[propName] = propValue;
      });
      continue;
    }

    const propValue = propType.instantiate(snapshot?.[propName], context);
    setParent(propValue, instance);
    instance[propName] = propValue;
  }

  if (identifierProp) {
    const id = instance[identifierProp];
    Object.defineProperty(instance, $identifier, { value: id });
    context.referenceCache.set(id, instance);
  }
};

which is to say, loop over the SomeClassModel.properties object which stores { key: SimpleType, createdDate: DateType, childObjects: MapType} etc, grabbing each key/value of the snapshot and using the type from the .properties to go from snapshot => live MQT object. This snapshot => live MQT object transform is what the .instantiate function on all our types is responsible for. Some examples:

on strings and numbers, do nothing
on dates, convert the incoming snapshot epoch number to a date with new Date()
on optional types, check for undefined / null and use the default if the snapshot is that, otherwise, instantiate the actual type
on map types, create an instance for each of the snapshot values, then assemble them into a QuickMap map

So, our compiler emits inline JS to do each of those things for the types we've fast-pathed, and then falls back on the existing .instantiate implementations for anything we haven't.
Here's what a compiled instantiation function looks like:
function buildCustomerInstantiator(model, imports) {
  const { QuickMap, $identifier } = imports;
  const v0 = model.properties["email"];
  const v1 = model.properties["phoneNumber"];

  return function InstantiateCustomer(instance, snapshot, context) {
    // optional type for type
    let snapshotValuetype = snapshot?.["type"];
    if (snapshotValuetype === undefined) {
      snapshotValuetype = "Customer";
    }

    instance["type"] = snapshotValuetype;

    // simple type for key
    instance["key"] = snapshot?.["key"];

    // simple type for firstName
    instance["firstName"] = snapshot?.["firstName"];

    // simple type for lastName
    instance["lastName"] = snapshot?.["lastName"];

    // instantiate fallback for email of type (string | null)
    instance["email"] = v0.instantiate(snapshot?.["email"], context, instance);

    // instantiate fallback for phoneNumber of type (string | null)
    instance["phoneNumber"] = v1.instantiate(
      snapshot?.["phoneNumber"],
      context,
      instance
    );

    // optional type for createdAt
    let snapshotValuecreatedAt = snapshot?.["createdAt"];
    if (snapshotValuecreatedAt === undefined) {
      snapshotValuecreatedAt =
        model.properties["createdAt"].defaultValueOrFunc();
    }

    instance["createdAt"] = snapshotValuecreatedAt;

    const id = instance["key"];
    instance[$identifier] = id;
    context.referenceCache.set(id, instance);
  };
}

The weird outer function is an artifact of the way that you are supposed to do eval in JS these days. eval is the sketchiest and slowest of the options -- better is to use new Function(<source>), which creates a new function in the global scope instead of the local one. This is good because you can't accidentally close over a bunch of stuff and leak it, but bad because if you do need context in the created function, you have to inject it weirdly. So, we actually create two functions: an outer one which establishes any variables we need in scope, like types or the QuickMap constructor, and then an inner function which is the one we actually run over and over and is the thing that needs to be very fast. Not super important but does make things look a bit weird.
There's a couple downsides of this whole approach:

the fast-path implementations could diverge from the slow-path, leading to hard to diagnose bugs
different type arrangements might expose end-user types to different paths over time
the fast-paths do a bit less less data validation, and may allow incorrect snapshots silently. We actually already accepted this for MQT, and rely on MST to do typechecking at write time, so MQT already doesn't do much runtime type validation at all, but the fast paths do even less.

Personally, I've found that the instantiate implementations haven't changed that much -- we've changed how we structure it all, but we haven't changed the core ideas at all. MQT since inception has had the same idea of an optional type, a map, of an array, of a model, etc, and I don't expect much churn in the list of primitives. I think we can expect some churn in how we implement them to be incredibly fast, but because the requirements are relatively set, I am ok adding brittleness to get that speed.

ping @angelini


mainへの反映

Coverage report
❌ An unexpected error occurred. For more details, check console
Error: The process '/usr/local/bin/npm' failed with exit code 1

Statements coverage not met for global: expected <=10 not covered statements, but got 4

St.:grey_question:
Category
Percentage
Covered / Total

🟢
Statements
80% (-20% 🔻)
4/5

🔴
Branches
50% (-50% 🔻)
1/2

🟢
Functions
100%
3/3

🟢
Lines
80% (-20% 🔻)
4/5

Show new covered files 🐣

St.:grey_question:
File
Statements
Branches
Functions
Lines

🟢
calc.js
80%
50%
100%
80%

Test suite run failed
Failed tests: 0/4. Failed suites: 0/1.
Report generated by 🧪jest coverage report action from d79e939b9c79999d644975008f018b16f5a04d1b


Bug: Discount not applied to items in subcategory

What are you trying to achieve?
I have created a discount through the saleor dashboard, set it to 10%, start date now and added my main category under which all other categories are. The product counter next to the category shows the correct amount of products in this category and all its subcategories.
When I now query the products in one of the subcategories I don't get the discount returned.
When I add the subcategory to the assigned categories in the discount, the discount is applied correctly through the graphql API.
Steps to reproduce the problem

Create a category "Category 1"
Create a subcategory "Category 1.1"
Create a product "Product 1" in category "Category 1"
Create a product "Product 1.1" in category "Category 1.1"
Create a discount and add "Category 1"
Query products through grapqhql including discount prices. "Product 1" will have a discount, "Product 1.1" not. When "Category 1.1" is added to the discount, the products in "Category 1.1" also get the discount applied.

What did you expect to happen?
The discount is applied to the category and all its subcategories. This is indicated by the "Products" count in the dashboard when adding categories to the discount.
Logs
Partial graphql response when only "Category 1" is added to the discount:
{
  "data": {
    "products": {
      "totalCount": 9,
      "edges": [
        {
          "node": {
            "id": "UHJvZHVjdDo4",
            "name": "Product 1",
            "pricing": {
              "priceRange": {
                "start": {
                  "gross": {
                    "amount": 13.5
                  }
                }
              },
              "discount": {
                "gross": {
                  "amount": 1.5,
                  "currency": "EUR"
                }
              }
            }
          }
        },
        {
          "node": {
            "id": "UHJvZHVjdDo5",
            "name": "Product 1.1",
            "pricing": {
              "priceRange": {
                "start": {
                  "gross": {
                    "amount": 15
                  }
                }
              },
              "discount": null
            }
          }
        },
        ...

response when both categories added to the discount:
{
  "data": {
    "products": {
      "totalCount": 9,
      "edges": [
        {
          "node": {
            "id": "UHJvZHVjdDo4",
            "name": "Product 1",
            "pricing": {
              "priceRange": {
                "start": {
                  "gross": {
                    "amount": 13.5
                  }
                }
              },
              "discount": {
                "gross": {
                  "amount": 1.5,
                  "currency": "EUR"
                }
              }
            }
          }
        },
        {
          "node": {
            "id": "UHJvZHVjdDo5",
            "name": "Product 1.1",
            "pricing": {
              "priceRange": {
                "start": {
                  "gross": {
                    "amount": 13.5
                  }
                }
              },
              "discount": {
                "gross": {
                  "amount": 1.5,
                  "currency": "EUR"
                }
              }
            }
          }
        },
        ...

Eligible categories shows 2 products for "Category 1" (one is directly in the category, one in the subcategory):

Environment
Saleor version: dashboard v3.18.0 core v3.18.9
OS and version: saleor platform, docker through colima on mac OS

Any response if this is intentional or not?


Zhang unordered review

A number of suggested changes for #26
Also a new failing unit test that needs to be fixed.

The suggestions/commits I wanted to keep have been cherry picked to #26. Thus this PR can be closed.


Can't get  disk info disk using container client

Verified on Version: 1.1.0-alpha @Revision: d1b3e6658c6805786abe795a080745c4aba39f0c


Not understanding how to setup software part.

Hi,
Currently I am trying to setup the software part of this project.
I am working with windows.
Currently I have setup 2 system variables:

cmake=C:\Program Files\CMake\bin\cmake
PLATFORM=teensy

I try to run CMake with the configurations you can find in the picture.

As you can see it gives an error.
I copied the ninja.exe file into the vive-diy-position-sensor. Actually I do not understand what to do with the ninja.exe
also, if i try to run cmake -G Ninja .. -DPLATFORM=Teensy, it gives me an error that the command cmake is not found.
I hope you can help me :)
thx!
Edit:
I think I got it figured out with the system variables. When run the command cmake -G Ninja .. -DPLATFORM=Teensy it still fails.
CMakeError.txt

Compiling the C compiler identification source file "CMakeCCompilerId.c" failed.
Compiler: C:/Program Files (x86)/GNU Tools ARM Embedded/6 2017-q2-update/bin/arm-none-eabi-gcc.exe
Build flags: -O2;-Wall;-ffunction-sections;-fdata-sections;-Wstack-usage=256;-mcpu=cortex-m4;-mthumb;-mfp16-format=ieee
Id flags:
The output was:
1
c:/program files (x86)/gnu tools arm embedded/6 2017-q2-update/bin/../lib/gcc/arm-none-eabi/6.3.1/../../../../arm-none-eabi/lib/thumb/v7e-m\libc.a(lib_a-exit.o): In function exit': exit.c:(.text.exit+0x16): undefined reference to _exit'
collect2.exe: error: ld returned 1 exit status
Compiling the CXX compiler identification source file "CMakeCXXCompilerId.cpp" failed.
Compiler: C:/Program Files (x86)/GNU Tools ARM Embedded/6 2017-q2-update/bin/arm-none-eabi-g++.exe
Build flags: -O2;-Wall;-ffunction-sections;-fdata-sections;-Wstack-usage=256;-mcpu=cortex-m4;-mthumb;-mfp16-format=ieee;-fno-rtti;-fsingle-precision-constant;-Woverloaded-virtual
Id flags:
The output was:
1
c:/program files (x86)/gnu tools arm embedded/6 2017-q2-update/bin/../lib/gcc/arm-none-eabi/6.3.1/../../../../arm-none-eabi/lib/thumb/v7e-m\libc.a(lib_a-exit.o): In function exit': exit.c:(.text.exit+0x16): undefined reference to _exit'
collect2.exe: error: ld returned 1 exit status
Determining if the C compiler works failed with the following output:
Change Dir: A:/hoge/17-18/ap vallet/vive-diy-position-sensor/build/CMakeFiles/CMakeTmp
Run Build Command:"A:/hoge/17-18/ap vallet/vive-diy-position-sensor/build/ninja.exe" "cmTC_b28c9"
[1/2] Building C object CMakeFiles/cmTC_b28c9.dir/testCCompiler.c.obj
[2/2] Linking C executable cmTC_b28c9
FAILED: cmTC_b28c9
cmd.exe /C "cd . && C:\PROGRA~2\GNUTOO~1\62017-~1\bin\AR19DD~1.EXE -O2 -Wall -ffunction-sections -fdata-sections -Wstack-usage=256 -mcpu=cortex-m4 -mthumb -mfp16-format=ieee  -O2 -Wl,--gc-sections,--relax,--defsym=__rtc_localtime=0 -mcpu=cortex-m4 -mthumb -mfp16-format=ieee -TA:/hoge/17-18/ap vallet/vive-diy-position-sensor/libs/cores/teensy3/mk20dx256.ld CMakeFiles/cmTC_b28c9.dir/testCCompiler.c.obj  -o cmTC_b28c9   && cd ."
AR19DD~1.EXE: error: vallet/vive-diy-position-sensor/libs/cores/teensy3/mk20dx256.ld: No such file or directory
ninja: build stopped: subcommand failed.

My OS is Windows and I use CMake 3.10.1.
Thx :)

Add Cmake path to environmental variable.
try disabling the anti virus/ windows defender.
execute cmake -G Ninja .. -DPLATFORM=Teensy command in cmd in build folder.


[SYSTEMML-769] Support for native BLAS and simplify deployment for GPU backend

This is a standing PR to facilitate discussion on whether or not, we should support native BLAS in SystemML. After discussion and after resolving the issues with deployment, we can decide whether to turn on this feature by default. Since I wanted feedback from community before proceeding ahead, I did not complete the PR. The remaining tasks are:

Generalize to other BLAS, not just MKL. This would also involve completing the CMake file.
Add other operations: conv2d_backward_*, etc.

I ran some preliminary performance experiments comparing conv2d with/without sparse+caching and with/without native BLAS. I provided fairly large memory budget (-Xmx20g -Xms20g -Xmn2048m -server) and used Open JDK 1.8 64-Bit Server VM. The script tested the performance of conv2d using four commonly used setups for 1000 iterations:
max_iterations = 1000
setup = $2
numFilters = -1
numChannels = -1
filterSize = -1
pad = -1
if(setup == 1) {
        numFilters = 20
        numChannels = 1
        filterSize = 5
        pad = 0
}
else if(setup == 2) {
        numFilters = 50
        numChannels = 20
        filterSize = 5
        pad = 0
}
else if(setup == 3) {
        numFilters = 20
        numChannels = 1
        filterSize = 3
        pad = 1
}
else if(setup == 4) {
        numFilters = 50
        numChannels = 20
        filterSize = 3
        pad = 1
}
else {
        stop('Incorrect setup (needs to be [1, 4]).')
}
imgSize = 28
n = 60000
X = rand(rows=n, cols=numChannels*imgSize*imgSize)
batch_size = 64
w = rand(rows=numFilters, cols=numChannels*filterSize*filterSize)
P = (imgSize + 2 * pad - filterSize)  + 1
foo = matrix(0, rows=n, cols=numFilters*P*P)
for(iter in 1:max_iterations) {
        beg = (iter * batch_size) %% n + 1
        end = min(n, beg + batch_size)
        X_batch = X[beg:end, ]
        n_batch = nrow(X_batch)
        convOut_1 = conv2d(X_batch, w, input_shape=[n_batch,numChannels,imgSize,imgSize], filter_shape=[numFilters,numChannels,filterSize,filterSize], padding=[pad,pad], stride=[1,1])
        foo = convOut_1
}
print(sum(foo))

To compile the native SystemML library, please use:
export MKLROOT=/opt/intel/mkl
export JAVA_HOME=....
# Please go to https://software.intel.com/en-us/articles/intel-mkl-link-line-advisor to find LINKER_OPTIONS and COMPILER_OPTIONS
export LINKER_OPTIONS=" -L${MKLROOT}/lib/intel64 -Wl,--no-as-needed -lmkl_rt -lpthread -lm -ldl"
export COMPILER_OPTIONS=" -m64 -I${MKLROOT}/include"
g++ -shared -fPIC -o libsystemml.so systemml.cpp -I. -I$JAVA_HOME/include -I$JAVA_HOME/include/linux -lm -fopenmp -O3 $LINKER_OPTIONS $COMPILER_OPTIONS

Please see below the results of the experiments. Both sparse and caching are disabled for the setup SystemML_native and SystemML_CP.
,Number of Iterations, Setup, Time in seconds
SystemML_native,1000,1,7.103096398
SystemML_CP,1000,1,6.498525426
SystemML_CP_WithCacheNSparseEnabled,1000,1,7.195620854
Tensorflow,1000,1,4.071731716
SystemML_native,1000,2,31.315343223
SystemML_CP,1000,2,81.769984552
SystemML_CP_WithCacheNSparseEnabled,1000,2,101.274622939
Tensorflow,1000,2,33.476548341
SystemML_native,1000,3,7.662274848
SystemML_CP,1000,3,6.355272119
SystemML_CP_WithCacheNSparseEnabled,1000,3,7.607337158
Tensorflow,1000,3,3.837932081
SystemML_native,1000,4,26.638438614
SystemML_CP,1000,4,49.716594505
SystemML_CP_WithCacheNSparseEnabled,1000,4,71.542244484
Tensorflow,1000,4,26.395180006

There are some additional overhead cost (such as initial compilation/validation, reuse of previously allocated but non-zeroed array, dynamic recompilation, GC, etc) which we have not yet optimized. These cost are beyond the scope of this PR and some of them are inherent to our design principles. We can work on them in a separate PR :)
@mboehm7 @bertholdreinwald @dusenberrymw @frreiss @prithvirajsen @fschueler @nakul02 @asurve   @deroneriksson   I understand the above experiments might not be sufficient to accept the change and would welcome your feedback on additional experiments/setups. I would also appreciate if some of you are willing to help me with these experiments too ;)
Here are the shapes of the matrix multiplication for the four setups:
Setup 1:
64 parallel matrix multiplication of shape (20, 25) %*% (25, 576) executed 1000 times.

Setup 2:
64 parallel matrix multiplication of shape (50, 500) %*% (500, 576) executed 1000 times.

Setup 3:
64 parallel matrix multiplication of shape (20, 25) %*% (25, 784) executed 1000 times.

Setup 4:
64 parallel matrix multiplication of shape (50, 500) %*% (500, 784) executed 1000 times.

I will provide an update soon comparing the results of the above matrix multiplications. If you are interested, here are the respective code path for the matrix multiplications:

CP: https://github.com/apache/incubator-systemml/blob/master/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixDNN.java#L327

Native: https://github.com/niketanpansare/incubator-systemml/blob/for_cpp/src/main/cpp/systemml.cpp#L163

Overall, I'm quite pleased to see that we can achieve ~equal performance to TensorFlow, particularly on case 4, which is the most probable scenario of the given tests.  26 seconds vs 71 seconds in SystemML today is a huge difference, and this is even with a small image size (2828) compared to say (256256*3), and an even larger number of channels for intermediate layers.
I'd also be interested in a comparison of a full network, such as the LeNet example in the SystemML-NN package for the various SystemML setups.
As for the BLAS integration, I think it would be a good idea to generically target a BLAS implementation, vs. a specific one.  A lot of people will be using OpenBLAS, for example.

I'd also be interested in a comparison of a full network, such as the LeNet example in the SystemML-NN package for the various SystemML setups.

Once we all agree that adding support for native BLAS is a good idea, we can add remaining operations and do comparison for a full network :+1:

As for the BLAS integration, I think it would be a good idea to generically target a BLAS implementation, vs. a specific one. A lot of people will be using OpenBLAS, for example.

I agree. In fact, the code is written against cblas_* interface to make this happen. To switch over to generic BLAS, all we need to change is:

Generalize two methods for setting the number of threads via conditional compilation: https://github.com/apache/incubator-systemml/pull/307/files#diff-601ed9dd7a60c7a3f26c91e73f68ea5cR48
Update the headers: https://github.com/apache/incubator-systemml/pull/307/files#diff-601ed9dd7a60c7a3f26c91e73f68ea5cR32
Edit CMake file: https://github.com/niketanpansare/incubator-systemml/blob/2dc5d862c107e6906c04a8ca66210a88b8547d2b/src/main/cpp/CMakeLists.txt

The results look great 👍
I agree with @dusenberrymw's comments about generalizing the implementation to plug in any BLAS. We cannot assume the availability of a Intel MKL on every system.

@bertholdreinwald @mboehm7 @fschueler  I ran experiments comparing the performance of matrix multiplication for 1000 iterations. The DML script used for this comparison is as follows:
X = matrix(0.1, rows=$1, cols=$2)
Y = matrix(0.2, rows=$2, cols=$3)
Z = matrix(0, rows=$1, cols=$3)
for(i in 1:1000) {
        Z = Z + X %*% Y
}
print(as.scalar(Z[1,1]))

The values provided to $1, $2 and $3 are 1 10 100 1000 2000 5000 10000, allowing us to test the performance of matrix multiplication for different shapes. I then extracted the time required for "ba+*" from the heavy hitters from each case and plotted them. The common dimension (i.e. $2) is plotted along the X-axis and the speedup time_for_ba+*_cp / time_for_ba+*_native is plotted along the Y-axis. The number of rows of X (i.e. $1) and the number of columns of Y (i.e. $3) are plotted using facets.
For example: The datapoint in red square refers to the speedup of native ba+* over LibMatrixMult for the case (1000, 5000) %*% (5000, 100). Here are the results for the cases in that box:
Setup, $1, $2, $2, $3, Time in seconds
Native,1000,1,1,100,0.499
CP,1000,1,1,100,0.377
Native,1000,10,10,100,0.522
CP,1000,10,10,100,0.955
Native,1000,100,100,100,0.472
CP,1000,100,100,100,1.780
Native,1000,1000,1000,100,1.286
CP,1000,1000,1000,100,6.849
Native,1000,2000,2000,100,2.196
CP,1000,2000,2000,100,12.234
Native,1000,5000,5000,100,5.870      <<<
CP,1000,5000,5000,100,28.050         <<<
Native,1000,10000,10000,100,10.249
CP,1000,10000,10000,100,55.804

It is important to note that we are competitive with the native library for matrix-vector multiplication.

I'm a bit puzzled about the experimental setting and results. Are you comparing here sparse-sparse matrix multiply and sparse-sparse element-wise addition? If so, are you also using Sparse BLAS and the same sparse matrix representation for both (e.g., CSR)? Which fraction of the reported time is actually spent in matrix multiplication (excluding output allocation etc)?

I am only comparing the time taken for dense matrix multiplication. The time reported is from our heavy hitters, so only includes ba+* time.

Time for output allocation is included in both native and CP. I only edited LibMatrixMult to redirect to native for fair comparison.

Hm, but why do you force dense matrix multiply on a sparse scenario of these shapes?
Anyway, could you please factor out a couple of things: on which scenarios did we apply multi-threading, how much time was spent in output allocation, how much time was spent in the actual matrix multiply, and how much time was spent in computing/maintaining the nnz in the output. Thanks.

May be I am missing something, why do you think the scenario should be sparse. If you prefer, I can test with random matrix instead of all 0.1 or 0.2. I didn't check each case but for a larger case, both Berthold and I noticed that all cores being used by CP. Also, I tried to ensure that the overhead (output allocation and nnz maintenance) happens for both the cases ... please see LibMatrixNative in this PR. I suspect if we factor out the overhead, speedup will be even more. Unfortunately, I won't be able to do any more experiments for at least another week, but I can share the setup if you or someone else want to take lead until then :)

sorry, my bad - when skimming the scenario, I mistakenly read it as rand of sparsity 0.1 and 0.2. In that case your can safely ignore my previous comments. However, it would still be useful to actually understand the details of where time goes and why the results show such a non-monotonic behavior.

just to clarify the issue of multi-threading: we have a parallelization threshold of 2MFLOPs as this is roughly the point where the creation of a thread pool is amortized. We experimented with a shared thread pool but the integration, especially with parfor, was not very clean so we did not put it into master. Please just annotate the cases where we don't use multi-threading help understand the large variation.

Can we add a "WIP" tag to this PR?

@mboehm7 I have created a branch with my experimental setup that annotates the cases where multi-threading was not enabled. Please see https://github.com/niketanpansare/incubator-systemml/blob/matmult_cpp_experiments/scripts/perftest/runMatMultExperiments.sh in case if you want to reproduce the result on a different machine.
I am running the experiments now and will provide an updated graph soon. Here is the preview of the results:
Java,SingleThreaded,1,1000,100,0.087730
IntelMKL,MultiThreaded,1,1000,1000,0.137696
OpenBLAS,MultiThreaded,1,1000,1000,1.190570
Java,SingleThreaded,1,1000,1000,0.619783
IntelMKL,MultiThreaded,1,1000,2000,0.220410
OpenBLAS,MultiThreaded,1,1000,2000,3.202002
Java,MultiThreaded,1,1000,2000,1.553647
IntelMKL,MultiThreaded,1,1000,5000,2.011847
OpenBLAS,MultiThreaded,1,1000,5000,9.932969
Java,MultiThreaded,1,1000,5000,2.930450
IntelMKL,MultiThreaded,1,1000,10000,7.010215
OpenBLAS,MultiThreaded,1,1000,10000,19.757398
Java,MultiThreaded,1,1000,10000,5.434230
IntelMKL,MultiThreaded,1,2000,1,0.023460
OpenBLAS,MultiThreaded,1,2000,1,0.019632
Java,SingleThreaded,1,2000,1,0.014827
IntelMKL,MultiThreaded,1,2000,10,0.065613
OpenBLAS,MultiThreaded,1,2000,10,0.033173
Java,SingleThreaded,1,2000,10,0.045123
IntelMKL,MultiThreaded,1,2000,100,0.118278
OpenBLAS,MultiThreaded,1,2000,100,0.239601
Java,SingleThreaded,1,2000,100,0.181720
IntelMKL,MultiThreaded,1,2000,1000,0.288750
OpenBLAS,MultiThreaded,1,2000,1000,3.235483
Java,MultiThreaded,1,2000,1000,1.489389
IntelMKL,MultiThreaded,1,2000,2000,2.614357
OpenBLAS,MultiThreaded,1,2000,2000,7.843344

@dusenberrymw @nakul02 @fschueler As an FYI, with some preliminary experiments, Java is outperforming OpenBLAS even with avx and fma flags (but not Intel MKL). The command used to compile with OpenBLAS is:
g++ -o /home/[user]/libsystemml.so systemml-cpp/systemml.cpp  -I/usr/lib/jvm/java-1.8.0-openjdk-<IP_ADDRESS>-1.b15.el7_2.x86_64/include -Isystemml-cpp -I/opt/openblas/include -I/usr/lib/jvm/java-1.8.0-openjdk-<IP_ADDRESS>-1.b15.el7_2.x86_64/include/linux -lopenblas -lpthread -lm -ldl -DUSE_OPEN_BLAS -L/opt/openblas/lib -fopenmp -O3 -shared -fPIC -mavx -mfma

@fschueler Either I am not compiling openblas in an optimal manner or your previous experiments might be using OpenCL ?

@mboehm7  Please see below the graphs comparing speedup with Intel MKL and  OpenBLAS respectively. As per your suggestion, I have marked the cases where multi-threading was not enabled
@fschueler  I think we donot see the speedup in OpenBLAS because it is likely not being compiled with AVX.

The R script used to generate above graphs:
t = read.csv("time1.txt", header=TRUE)
t = subset(t, BLAS != "IntelMKL" & Time > 0)
temp = subset(t, BLAS == "Java")
for(i in 1:nrow(temp)) {
  M1 = temp[i,]$M
  N2 = temp[i,]$N
  K1 = temp[i,]$K
  temp[i,]$Time = subset(t, BLAS == "Java" & M == M1 & N == N2 & K == K1)$Time /  subset(t, BLAS != "Java" & M == M1 & N == N2 & K == K1)$Time
}
qplot(N, Time, data=temp, color=IsSingleThreaded, facets=M~K, xlab="Common dimension (N)", ylab="Speedup", main="Matrix multiplication (Speedup = CP/OpenBLAS)")

@nakul02 @fschueler @dusenberrymw Can you help me with Mac command for OpenBLAS and Intel MKL ? https://github.com/niketanpansare/incubator-systemml/blob/for_cpp/src/main/python/setup.py#L70

@fschueler I used following steps to compile OpenBLAS:
git clone https://github.com/xianyi/OpenBLAS
cd OpenBLAS
make FC=gfortran
sudo make PREFIX=/opt/openblas install

Also, the output of getarch utility included in OpenBLAS directory is as follows:
$ ./getarch 0
CORE=HASWELL
LIBCORE=haswell
NUM_CORES=24
HAVE_MMX=1
HAVE_SSE=1
HAVE_SSE2=1
HAVE_SSE3=1
HAVE_SSSE3=1
HAVE_SSE4_1=1
HAVE_SSE4_2=1
HAVE_AVX=1
HAVE_FMA3=1

To summarize, I tried following with openblas, but still saw performance degradation:

Compiling OpenBLAS from source:
git clone https://github.com/xianyi/OpenBLAS
cd OpenBLAS
make FC=gfortran
sudo make PREFIX=/opt/openblas install

Using released OpenBLAS:
sudo yum install openblas
sudo ln -s /lib64/libopenblas.so.0 /usr/lib/libopenblas.so

Also, commented any threading logic in systemml.cpp to rule out possibility of bug due to extension APIs:
    //if(NUM_THREADS == -1) {
    //      NUM_THREADS = openblas_get_num_threads();
    //}
    // openblas_set_num_threads(NUM_THREADS);

Even though I was using newer Centos 7 and gcc 4.8 (both supporting avx instructions), I also tried updating binutils and reinstalling OpenBLAS as per https://github.com/xianyi/OpenBLAS/wiki/faq#binutils

My guess is that this is due to a performance bug in OpenBLAS on multi-socket machine: https://github.com/xianyi/OpenBLAS/issues/611
$ cat /proc/cpuinfo | grep "physical id" | sort -u | wc -l
2

Please ignore my earlier two comments about OpenBLAS. I was able to resolve the performance issue and have got this PR to functionally complete state. Here are the features that are included as part of this PR:

Automatic detection of BLAS and GPU as well as including the correct dependency. Please see http://niketanpansare.github.io/incubator-systemml/accelerator for usage.

No additional steps of including JCuda java and lib required.
This setup will work in hybrid cluster environment (with/without gpus, different blas) as well.
We support GPU on Linux (x86_64 and powerpc), Mac (x86_64), Windows (x86_64) and BLAS on Linux (x86_64), Mac (x86_64), Windows (x86_64). For other setups, we revert back to LibMatrixMult and non-GPU backend.

Support BLAS for matrix multiplication, conv2d and conv2d_backward_data.

Performance comparison of LibMatrixMult with MKL and OpenBLAS.

@nakul02 @mboehm7 @bertholdreinwald @dusenberrymw @deroneriksson @asurve @gweidner @fschueler   Can you please try following script in your environment and comment back on this PR with log lines with prefix INFO accelerator. This should help us identify the setup on which this PR is tested.
git clone https://github.com/niketanpansare/incubator-systemml.git
cd incubator-systemml
git checkout for_cpp
mvn clean package -P distribution
pip install target/systemml-0.12.0-incubating-SNAPSHOT-python.tgz
pyspark
>>> from systemml import random
>>> m1 = random.uniform(size=(1000,1000))
>>> m2 = random.uniform(size=(1000,1000))
>>> m3 = m1.dot(m2).toNumPy()

Here are the logs from my setups:

machine 1: installed openblas, no gpu, linux
machine 2: installed openblas, mkl , cuda 8, cudnn 5.1, linux

On machine 1:
16/12/22 11:21:38 INFO accelerator.LibraryLoader: Unable to load MKL:no mkl_rt in java.library.path
16/12/22 11:21:38 INFO accelerator.BLASHelper: Found BLAS: openblas
16/12/22 11:21:38 INFO accelerator.BLASHelper: Successfully loaded systemml library with openblas

On machine 2 (SYSTEMML_GPU and SYSTEMML_BLAS not set):
16/12/22 14:30:46 INFO accelerator.BLASHelper: Found BLAS: mkl
16/12/22 14:30:46 INFO accelerator.BLASHelper: Successfully loaded systemml library with mkl
16/12/22 14:30:52 INFO accelerator.JCudaHelper: Total number of GPUs on the machine: 2
16/12/22 14:30:52 INFO accelerator.JCudaHelper: GPU is enabled

On machine 2 (SYSTEMML_GPU not set and SYSTEMML_BLAS set to openblas):
16/12/22 14:33:07 INFO accelerator.BLASHelper: Found BLAS: openblas
16/12/22 14:33:07 INFO accelerator.BLASHelper: Successfully loaded systemml library with openblas
16/12/22 14:33:12 INFO accelerator.JCudaHelper: Total number of GPUs on the machine: 2
16/12/22 14:33:12 INFO accelerator.JCudaHelper: GPU is enabled

On machine 2 (SYSTEMML_GPU set to none and SYSTEMML_BLAS set to openblas):
16/12/22 14:34:05 INFO accelerator.BLASHelper: Found BLAS: openblas
16/12/22 14:34:05 INFO accelerator.BLASHelper: Successfully loaded systemml library with openblas
16/12/22 14:34:05 INFO accelerator.JCudaHelper: Not loading JCUDA as SYSTEMML_GPU=none

On machine 2 (SYSTEMML_GPU set to none and SYSTEMML_BLAS set to none):
16/12/22 14:34:54 INFO accelerator.BLASHelper: Not loading native BLAS as SYSTEMML_BLAS=none
16/12/22 14:34:54 INFO accelerator.JCudaHelper: Not loading JCUDA as SYSTEMML_GPU=none

If you have few more free cycles, please run the microbenchmarks: https://github.com/niketanpansare/incubator-systemml/tree/for_cpp/scripts/perftest/microbenchmarks

All tests passed here.

@niketanpansare W.r.t. BLAS on OS X / macOS, currently, the preferred and default BLAS implementation is Apple Accelerate.  Like C-BLAS, Atlas, OpenBLAS, Intel MKL, etc, Accelerate implements the BLAS API, and in terms of performance, Accelerate is generally the fastest BLAS implementation on OS X / macOS. Also, just to be clear, Accelerate is part of OS X / macOS, and does not require any installation.
For example, a pip install numpy on OS X / macOS will automatically be linked to Accelerate:
import numpy as np
np.__config__.show()

blas_mkl_info:
  NOT AVAILABLE
openblas_info:
  NOT AVAILABLE
atlas_3_10_blas_threads_info:
  NOT AVAILABLE
atlas_3_10_blas_info:
  NOT AVAILABLE
atlas_blas_threads_info:
  NOT AVAILABLE
atlas_blas_info:
  NOT AVAILABLE
blas_opt_info:
    extra_compile_args = ['-msse3', '-I/System/Library/Frameworks/vecLib.framework/Headers']
    extra_link_args = ['-Wl,-framework', '-Wl,Accelerate']
    define_macros = [('NO_ATLAS_INFO', 3), ('HAVE_CBLAS', None)]
openblas_lapack_info:
  NOT AVAILABLE
lapack_mkl_info:
  NOT AVAILABLE
atlas_3_10_threads_info:
  NOT AVAILABLE
atlas_3_10_info:
  NOT AVAILABLE
atlas_threads_info:
  NOT AVAILABLE
atlas_info:
  NOT AVAILABLE
lapack_opt_info:
    extra_compile_args = ['-msse3']
    extra_link_args = ['-Wl,-framework', '-Wl,Accelerate']
    define_macros = [('NO_ATLAS_INFO', 3), ('HAVE_CBLAS', None)]

@niketanpansare I ran the quick test above, and I did not see any INFO accelerator ... lines.  I did have the following though on OS X / macOS, no GPU:
17/01/09 15:36:17 INFO SparkContext: Added JAR /usr/local/lib/python2.7/site-packages/systemml/systemml-java/systemml-0.12.0-incubating-SNAPSHOT.jar at http://localhost:50037/jars/systemml-0.12.0-incubating-SNAPSHOT.jar with timestamp<PHONE_NUMBER>792
17/01/09 15:36:17 INFO SparkContext: Added JAR /usr/local/lib/python2.7/site-packages/systemml/systemml-java/systemml-accelerator.jar at http://localhost:50037/jars/systemml-accelerator.jar with timestamp<PHONE_NUMBER>848
17/01/09 15:36:35 INFO LibraryLoader: Unable to load MKL:no mkl_rt in java.library.path
17/01/09 15:36:35 INFO LibraryLoader: Unable to load OpenBLAS:no openblas in java.library.path
17/01/09 15:36:36 INFO LibraryLoader: Unable to load CUDA:no cuda in java.library.path

Can you double check if OpenBLAS is available to Java ? https://github.com/niketanpansare/incubator-systemml/blob/9f325e1ac7155bf6a1cf98fc8e314ac339a198a0/docs/accelerator.md#frequently-asked-questions
Also, I agree with you that we should support Accelerate once this PR is in.

Well, I didn't install OpenBLAS, but I'll look into it on a Linux box.  As for the rest of this PR, the systemml-accelerator.jar idea is awesome, but I don't think we will be able to ship a release that contains that JAR due to all of the binary files included.  Thoughts?

Thanks @dusenberrymw  ....  We have three options:

Seperate project option: We have a seperate project https://github.com/niketanpansare/systemml-accelerator and

Provided dependency:   We host systemml-accelerator.jar on maven central once this PR is accepted and provide the link in the documentation. I can also create a pip installer for that.
Compile dependency: Here we include a fat artifact where we compile systemml-accelerator.jar  into SystemML. Thereby, no requiring an release of systemml-accelerator.jar.

One project option: We include the files from https://github.com/niketanpansare/systemml-accelerator into our project and have a separate profile to compile libraries conditionally.

Closing this PR in favor of https://github.com/apache/incubator-systemml/pull/344 and https://github.com/apache/incubator-systemml/pull/291


Investigate why some files are being identified as .exe

Other tooling identifies these as MP4 which is really strange. Quite far from EXE
I'll beat things around a bit.


[DEV] Adding org-notes about the native commands

Hey @abhi18av here goes my notes with tall the commands mentioned on the standard output, the commands using parallel and full seems to be the same, but there are steps that uses mtbseq tools, not external tools. I just wrote what mtbseq does in the analysis process sense.
This Pull Request closes #29
Feel free to discuss these notes, and also to add yours. If you think that there's something missing, please tell me and I'll proceed with the changes,
Hope we have a merge!

Ok I'll add that

I actually have notes for the process for parallel run. I'll add notes about each process.

hey! @abhi18av I've proceeded with the changes as requested, now we have a markdown file with the required content


TypeError: Cannot call method 'replace' of undefined

When the hook is triggered the following error occurred and the build is not triggered:
./jekyll-hook.js
connect deprecated urlencoded.verify: use body-parser module for verify node_modules/express/node_modules/connect/lib/middleware/bodyParser.js:55:21
connect deprecated multipart: use parser (multiparty, busboy, formidable) npm module instead node_modules/express/node_modules/connect/lib/middleware/bodyParser.js:56:20
connect deprecated limit: Restrict request size at location of read node_modules/express/node_modules/connect/lib/middleware/multipart.js:86:15
connect deprecated json.verify: use body-parser module for verify node_modules/express/node_modules/connect/lib/middleware/bodyParser.js:57:15
Listening on port 8080
TypeError: Cannot call method 'replace' of undefined
    at /opt/jekyll-hook/jekyll-hook.js:52:32
    at pop (/opt/jekyll-hook/node_modules/queue-async/queue.js:24:14)
    at Object.q.defer (/opt/jekyll-hook/node_modules/queue-async/queue.js:55:11)
    at port (/opt/jekyll-hook/jekyll-hook.js:45:11)
    at callbacks (/opt/jekyll-hook/node_modules/express/lib/router/index.js:164:37)
    at param (/opt/jekyll-hook/node_modules/express/lib/router/index.js:138:11)
    at pass (/opt/jekyll-hook/node_modules/express/lib/router/index.js:145:5)
    at Router._dispatch (/opt/jekyll-hook/node_modules/express/lib/router/index.js:173:5)
    at Object.router (/opt/jekyll-hook/node_modules/express/lib/router/index.js:33:10)
    at next (/opt/jekyll-hook/node_modules/express/node_modules/connect/lib/proto.js:174:15)

NodeJS v0.10.38

Hi @sammcj , this looks like the server is just getting an unhandled event type that doesn't contain the ref property. When setting up the webhook, you may just want to select only the PushEvent which should resolve the issue.

I am actually having this issue as well. My configuration:

centos 6 w/ nvm & node 0.12.7
nginx 1.8
Webhook on github configured to only send push event
I do get a 202 back from the server, but this is happening in console:

TypeError: Cannot read property 'replace' of undefined
    at /var/www/jekyll-hook/jekyll-hook.js:52:31
    at pop (/var/www/jekyll-hook/node_modules/queue-async/queue.js:24:14)
    at Object.q.defer (/var/www/jekyll-hook/node_modules/queue-async/queue.js:55:11)
    at /var/www/jekyll-hook/jekyll-hook.js:45:11
    at callbacks (/var/www/jekyll-hook/node_modules/express/lib/router/index.js:164:37)
    at param (/var/www/jekyll-hook/node_modules/express/lib/router/index.js:138:11)
    at pass (/var/www/jekyll-hook/node_modules/express/lib/router/index.js:145:5)
    at Router._dispatch (/var/www/jekyll-hook/node_modules/express/lib/router/index.js:173:5)
    at Object.router (/var/www/jekyll-hook/node_modules/express/lib/router/index.js:33:10)
    at next (/var/www/jekyll-hook/node_modules/express/node_modules/connect/lib/proto.js:174:15)


Pub/Sub: TransportChannelProvider MaxInboundMessageSize default value not sufficient

Describe the bug
Pub/Sub subscribers can receive up to 20MiB of data, but the auto configuration overrides the TransportChannelProvider of the Pub/Sub library without specifying setMaxInboundMessageSize which keeps the default value of 4 MiB. Therefore, it is very likely that an user is facing exceptions: RESOURCE_EXHAUSTED: gRPC message exceeds maximum size 4194304 when high volumne of data is processed
Here the default settings for subscribes can be seen:
https://github.com/googleapis/java-pubsub/blob/8cf77ee3b31356f342942a9ffaa61db2d0686769/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/stub/SubscriberStubSettings.java#L282
From my point of view spring-cloud-gcp should provide correct default values here:
https://github.com/spring-cloud/spring-cloud-gcp/blob/2adf4328c819a634a3ad7a1627426badf198c4fb/spring-cloud-gcp-autoconfigure/src/main/java/org/springframework/cloud/gcp/autoconfigure/pubsub/GcpPubSubAutoConfiguration.java#L387
I'm aware that https://github.com/spring-cloud/spring-cloud-gcp/issues/1808 is open and this settings topic might be solved more comprehensive (there also might be a differentiation also within pub/sub), but this might take a while and as an quick improvement I'd suggest to set a proper limit of at least 20 MiB in the auto configuration. This should improve the user experience for now. I'm open to contribute this change.
Sample
This 4MiB limit was already discussed in some places here. For example: https://github.com/spring-cloud/spring-cloud-gcp/issues/2476
A workaround written in Kotlin reusing the subscribers default settings:
@Bean
@ConditionalOnProperty(prefix = "spring.cloud.gcp.pubsub", name = ["emulator-host"], havingValue = "false", matchIfMissing = true)
fun transportChannelProvider(gcpPubSubProperties: GcpPubSubProperties): TransportChannelProvider {
   return SubscriberStubSettings.defaultGrpcTransportProviderBuilder()
               .setKeepAliveTime(Duration.ofMinutes(gcpPubSubProperties.keepAliveIntervalMinutes.toLong()))
               .build()
}

@f-loris Generally we defer to the underlying client library for defaults. So, it might make more sense to propose this change there. On our side, we should probably add a property to GcpPubSubProperties to easily change MaxInboundMessageSize in app properties.

@meltsufin I totally agree that it should defer to the underlying client library's defaults, but spring-cloud-gcp is overriding these defaults by setting it's own TransportChannelProvider. As I posted above, for Subscribers the default setting is already set properly in the client library: https://github.com/googleapis/java-pubsub/blob/8cf77ee3b31356f342942a9ffaa61db2d0686769/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/stub/SubscriberStubSettings.java#L282
As I also mentioned above, these settings even ifferentiate within the client library as for example Publisher have a different default setting: https://github.com/googleapis/java-pubsub/blob/8cf77ee3b31356f342942a9ffaa61db2d0686769/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/stub/PublisherStubSettings.java#L249

@f-loris I think a separate TransportChannelProvider for publisher and subscriber with correct defaults is the way to go.
I'll raise a PR for this.


Run tests on python 3.8 in CI - [merged]

In GitLab by @wtbarnes on Oct 25, 2019, 16:01
Merges test-python-3.8 -> master
Fixes #39

In GitLab by @wtbarnes on Oct 25, 2019, 16:02
enabled an automatic merge when the pipeline for 92d5b693d235c342886920f34a88aa92656f4ffa succeeds

In GitLab by @wtbarnes on Oct 25, 2019, 23:55
enabled an automatic merge when the pipeline for 490fdc26fafc1f3fec785eaf69a8ad04dd8e8edb succeeds

In GitLab by @wtbarnes on Oct 26, 2019, 24:02
mentioned in commit 72d34fd2b08ab474d81bb3ab20d3985b2075a9c5

In GitLab by @wtbarnes on Oct 26, 2019, 24:02
merged

In GitLab by @wtbarnes on Oct 25, 2019, 23:53
Marking these tests as allowed to fail now. There is an issue building scipy wheels on Python 3.8 at the moment. Once these allowed failures start passing, this can be removed.

In GitLab by @wtbarnes on Oct 25, 2019, 23:50
added 1 commit
490fdc26 - allow py38 tests to fail for now
Compare with previous version

In GitLab by @wtbarnes on Mar 23, 2020, 13:30
mentioned in commit 48aa5be4e275d5ec9536e911fe7ff336df95b61e


New maintainers

Hello all,
scikit-optimize has seen a lot of new contributions recently from @holgern. He contacted the current maintainers (@glouppe, @iaroslav-ai and @MechCoder) asking if he could help out with maintenance.
Welcome Holger!
The guidelines for interacting and contributing remain the same.
A main task for maintaining this project is around growing the number of people who actively contribute to the maintenance of the project in order to keep the level of quality high. This is a hard task! Depending on what part of the history of the project you look at the past maintainers did a good or a bad job at this. I look forward to seeing how new maintainers tackle this challenge and keep growing the community.
As part of this I would be happy to see more people creating PRs and increasing their involvement so that new people can join Holger in maintaining this project.

I would like to nominate myself as a new maintainer. :raised_hand_with_fingers_splayed:
It's a real shame if not wholly unacceptable to see a such a well-rounded drop-in library stall unmaintained, accumulating untriaged issues even to the point of incompatibilities with its dependencies and downgrades as the recommended course of action!
Having just recently evaluated several options in the Python hyperparameter optimization space, I found scikit-optimize the one to go with for one of my projects, so I have some direct interest in it remaining maintained and useful.
If you'll consider my application, please see examples of my reasoning, work, and interaction already in the issue tracker, with more available obviously in other projects I work on.
As a new maintainer, I'd begin by:

[ ] porting CI to GitHub Actions (can't have CI broken or taking hours to complete),
[ ] triaging existing issues and PRs (assigning labels like you guys have till late 2017), tidying up and hopefully making everyone again somewhat more interested in contributing.

Thanks for your consideration.
cc: @scikit-optimize, @betatim, @glouppe, @iaroslav-ai, @MechCoder, @holgern

I would like to nominate myself as a new maintainer. :raised_hand_with_fingers_splayed:
It's a real shame if not wholly unacceptable to see a such a well-rounded drop-in library stall unmaintained, accumulating untriaged issues even to the point of incompatibilities with its dependencies and downgrades as the recommended course of action!
Having just recently evaluated several options in the Python hyperparameter optimization space, I found scikit-optimize the one to go with for one of my projects, so I have some direct interest in it remaining maintained and useful.
If you'll consider my application, please see examples of my reasoning, work, and interaction already in the issue tracker, with more available obviously in other projects I work on.
As a new maintainer, I'd begin by:

[ ] porting CI to GitHub Actions (can't have CI broken or taking hours to complete),
[ ] triaging existing issues and PRs (assigning labels like you guys have till late 2017), tidying up and hopefully making everyone again somewhat more interested in contributing.

Thanks for your consideration.
cc: @scikit-optimize, @betatim, @glouppe, @iaroslav-ai, @MechCoder, @holgern

Hi @kernc 👋
thanks for taking the time to find, evaluate and then start helping out here!
It would be great to meet you to chat a bit. For me a large part of maintaining a project is about the social side of things. Not just technical skills. How could we make this happen given everyone has a busy life, timezones, etc?

Hi @kernc 👋
thanks for taking the time to find, evaluate and then start helping out here!
It would be great to meet you to chat a bit. For me a large part of maintaining a project is about the social side of things. Not just technical skills. How could we make this happen given everyone has a busy life, timezones, etc?

I got an interview! :raised_hands: Sure thing. It shouldn't be too hard to sync — we're all keeping busy, but at least we're mostly locked in. :laughing: Let me send you an email.

I got an interview! :raised_hands: Sure thing. It shouldn't be too hard to sync — we're all keeping busy, but at least we're mostly locked in. :laughing: Let me send you an email.

@betatim @kernc @holgern (@glouppe, @iaroslav-ai and @MechCoder)
sorry for the ping - but i'd like to bring to your attention that scikit-optimize (this project) is (once again) in a pretty desolate state.
There's several breaking compatibility issues with recent numpy / scikit-learn versions  (#1171, #1137) - both of which would have fixing PR's available (#1187, #1184).
Unfortunately, CI is also failing due to the image used having reached end of life.
There's also contributions available (#1074) which would probably be fixing this (and seem very close to be working, if it's not good already).
I think the parts (community contributions, community interest) are available for this project to succeed - but it'll need some love for sure.

What way forward would you propose @xmatthias? I think from the side of the people you pinged no one has the time to either actively work on this project or find new maintainer(s). Which makes it difficult to see a way forward.

Well that's the problematic part, really.
if I'd have a solution, i'd have proposed that above.
Please treat the below as a rough set of ideas / my opinions only.

In my (naiive) understanding, scikit-optimize was part of the scikit-learn foundation / ecosystem - which might might make it eligible to either funding, or time-based contributions by scikit-learn funded contributors to at least keep basic compatibility (no new features ...).
But maybe / probably i'm wrong considering that this is a separate github org - so might be completely unrelated to scikit-learn (i might also be wrong about funding as part of scikit-learn - i'm not really familiar with this entity as an organization).

Ideally, an issue with "call for new maintainers" - which can then be vetted somewhat (you'll not want some random guy from the internet you never seen and never commited anything ... - maybe based on historic contributions to this / other similar packages, but criteria can essentially be anything) should be done.
Now based on your above comment, we doubt anyone will have time for this  - which essentially means that we have

no active support to keep / restore compatibility
no time to find successors

Which essentially declares the project dead - at least for now.

Which makes it difficult to see a way forward.

If that's the conclusion, then I think this should be clearly stated at the top of the project's readme, providing projects depending on scikit-optimize clarity on the project's future (or lack thereof) - which can aid / simplify their decision to either move to supported optimization frameworks, or to accept the drawbacks.
This doesn't mean scikit-optimize doesn't work as is, given the constraints on older scikit-learn and numpy versions - but it'll clarify expectations and most likely will also reduce frustration in issues - where right now, simply no answer is given.
With such a statement, it's clear that no answer should be expected (which won't mean there will be none - just that it's not very likely).


⚠ Blutopia has degraded performance

In c35698b, Blutopia ($BLU) experienced degraded performance:

HTTP code: 200
Response time: 4141 ms

Resolved: Blutopia performance has improved in d90495a after 7 minutes.


'Black Screen' / Unable to take picture on Ipad mini 2, IOS 9.2

Gallery works fine. When I press the TakePicture button the camera shows but the screen is black. I tried switching cameras, would not help. I did notice some yellow outlines as if the image recognition and identification system was running. When I tried to take a picture nothing happened.
IPad Mini 2, running on IOS 9.2

I have the same.

Same here on iphone5

Well, investigating a bit more, I think this can happen when the user does not have permissions to access the camera. Some times I get the permission request. Some times I don´t (even reinstalling the app). Not sure why

This issue was moved to jamesmontemagno/MediaPlugin#25


Fixed Checkstyle Errors

Fixed majority of checkstyle errors throughout all current files.

That's great! Good to see that we've fixed almost all of the Checkstyle errors, with only few lines/errors being left due to how we may refactor/change the purpose in the future!


Change the background to White

Title ^

I will do this! thanks

@stephdavl  Perfect thanks!

#4

Thanks everyone!


FailedNodeAllocatableEnforcement on cgroups-per-qos false

Is this a request for help?:
Yes
Is this an ISSUE or FEATURE REQUEST? (choose one):
Issue
What version of acs-engine?:
As of now all shipping with PR #1960

Orchestrator and version (e.g. Kubernetes, DC/OS, Swarm)
Tested with Kubernetes 1.8.4, 1.8.6, 1.8.7
What happened:
Kubelets constantly file events like this:
Warning  FailedNodeAllocatableEnforcement  49s (x3 over 2m)  kubelet, k8s-master-56038831-0  Failed to update Node Allocatable Limits "": failed to set supported cgroup subsystems for cgroup : Failed to set config for supported subsystems : failed to write<PHONE_NUMBER> to memory.limit_in_bytes: write /var/lib/docker/overlay2/05c98c88dcf284951773f777b09d37101b1eed0eb43b7e5295ee0600f4390f1e/merged/sys/fs/cgroup/memory/memory.limit_in_bytes: invalid argument

What you expected to happen:
This warning not to pop up, i.e., Normal   NodeAllocatableEnforced           4s                kubelet, k8s-master-56038831-0  Updated Node Allocatable limit across pods instead
How to reproduce it (as minimally and precisely as possible):
Deploying cluster using this model (I removed some secrets and ssh key):
{
  "apiVersion": "vlabs",
  "properties": {
    "orchestratorProfile": {
      "orchestratorType": "Kubernetes",
      "orchestratorRelease": "1.8",
      "orchestratorVersion": "1.8.6",
      "kubernetesConfig": {
        "kubernetesImageBase": "gcrio.azureedge.net/google_containers/",
        "DockerBridgeSubnet": "<IP_ADDRESS>/16",
        "kubeletConfig": {
          "--eviction-hard": "memory.available<500Mi,nodefs.available<10%,nodefs.inodesFree<5%",
          "--system-reserved": "memory=1.5Gi"
        }
      }
    },
    "masterProfile": {
      "count": 1,
      "dnsPrefix": "qos-cgroups-test",
      "vmSize": "Standard_D2_v3",
      "storageProfile": "ManagedDisks",
      "oauthEnabled": false,
      "distro": "ubuntu",
      "vnetCidr": "<IP_ADDRESS>/16"
    },
    "agentPoolProfiles": [
      {
        "name": "agents",
        "count": 1,
        "vmSize": "Standard_E2s_v3",
        "osType": "Linux",
        "availabilityProfile": "AvailabilitySet",
        "storageProfile": "ManagedDisks",
        "distro": "ubuntu"
      }
    ],
    "linuxProfile": {
      [...]
    },
    "servicePrincipalProfile": {
      [...]
    }
  }
}

Afterwards running kubectl describe node <NODE> on the new cluster - the described event should pop up.
Manually changing /etc/default/kubelet and changing --cgroups-per-qos to true instead of false makes the event disappear. However, I have not further investigated whether or not this has other side effects.
Hence, I have two questions: a) Am I the only person experiencing this? And, b) what is the rational behind changing this argument from true (as per default) to false and making it static at the same time?
Also, am I missing something? As far as I can tell I am not doing anything specific, this should be a very plain setup?
Anything else we need to know:

+1

cc @karataliu @feiskyer

I am getting the same issue on an K8s 1.9.2 cluster using acs-engine 0.12.5 engine. In fact over 19K time over 3 days. @yastij @karataliu @feiskyer

@jcharlytown We are observing the same issue. According to the docs the  --cgroups-per-qos flag needs to be enabled – https://kubernetes.io/docs/tasks/administer-cluster/reserve-compute-resources/#enabling-qos-and-pod-level-cgroups

cc @arnaudmz

PR #2310 aims to fix this

Thanks everyone for fixing this. +1!

I just checked the latest release - it seems this fix is not included. Is there any specific reason?

cc @jackfrancis

@jcharlytown @yastij Are you able to build from master to deploy clusters? If there is an overwhelming desire for a patch release we can include this fix in such a release this week.

@jackfrancis, I can build from master, yes. I was just wondering why that was in general. To understand the procedures here might come in handy in the future as we are currently evaluating what tool chain to use to manage our clusters. Can you comment on why this was not included?

@jackfrancis - would be really nice, thanks !

We generate a patch release when there is a wide-impacting fix that we want to encourage folks to opt into. This is why it's important to vote when you want to see something sooner than the next minor release. So if there's a v0.13.2, we'll include this in it. :)
Additionally in the future we plan to patch release when a new Kubernetes release is published to make it easier to deploy new clusters (without requiring users to build from master).

SGTM


fix: golangci-lint error

Which issue(s) this PR fixes:
https://github.com/longhorn/longhorn/issues/7448
What this PR does / why we need it:
Special notes for your reviewer:
Additional documentation or context

Just one feedback about when dealing an error but it's not critical and unhandled or it can be recovered eventually, suggest logging with warn level instead of error.

Thank you. I change the log level to warning.


Adding basic support for specifying create timeout.

For me the Vagrant VMs were never building before apparently being timed out by terraform.   My golang knowledge is rudimentary, to say the least, but I examined how this was implemented for another provider and got a version that appears to work (respecting the configured timeout values).
Happy to update the docs too, if you're amenable to this PR.

Hey @hozn, thanks for the PR! Do you know if the timeouts are coming from terraform, or from vagrant? According to the terraform docs the default timeout should have been 20 minutes whereas I see you defaulted to 2 minutes in your PR. Are you using the timeouts {} directive in your terraform configuration?

Hmm ... I am using that directive and it seemed to address an issue, but it sounds like I need to investigate further what might have been timing out and whether that was a coincidence!

Hmm, ya, I'd like to get to the bottom of the problem to make sure we've got the right solution =)


Add Banner Tracking Link to robots.txt

The tracking Link from the Banner module can cause errors in webmaster console, so we should disallow them.

1. Why is this change necessary?
2. What does this change do, exactly?
Adding /tracking to robots.txt
3. Describe each step to reproduce the issue or behaviour.
...
4. Please link to the relevant issues (if any).
5. Which documentation changes (if any) need to be made because of this PR?
6. Checklist

[ ] I have written tests and verified that they fail without my change
[x ] I have squashed any insignificant commits
[ x] This change has comments for package types, values, functions, and non-obvious lines of code
[ ]x I have read the contribution requirements and fulfil them.

Hello,
thank you for creating this pull request.
I have opened an issue on our Issue Tracker for you. See the issue link: https://issues.shopware.com/issues/SW-24498
Please use this issue to track the state of your pull request.

Hi @aherking, thank you very much for your contribution! 💙


Please release under the MIT license

Hey,
I'm wrapping this in a CLI tool, and would like to release this open-source-ly.
If you would like to see contributions and maintenance coming from the open source Python community, please release this under an open source license (I enthusiastically recomment MIT).
Cheers,
Shay

Done in https://github.com/pyvista/numpydoc-validation/commit/7303e7b7cd5d7cf10aad68bf6f8135739a1157b0, should have added that a while ago.

Also, I'd love to be able to resolve #2 by your CLI tool. Please let me know when you release that so I can include part of it here, or you can feel free to submit a PR (and I'll add CI).

Great, thank you!
I wish I could find time for either a PR or wrapping this up, but because your code ended up not working for me I've ended up writing a quick script for my own use:
https://github.com/pdpipe/pdpipe/blob/master/dev/scripts/numpydoc_validate.py


The description '(inferred)' of Access Tier doesn't display for one uploaded blob

Storage Explorer Version: 1.9.0
Build: master_20190828.15
Platform/OS: Windows 10/ Linux Ubuntu 18.04/macOS High Sierra
Architecture: ia32/x64
Regression From: Not a regression
Steps to reproduce:

Make sure AzCopy is unchecked.
Expand one StorageV2 account -> Blob Containers.
Create a new blob container -> Upload one blob.
Check the value of the blob's Access Tier in the editor.

Expect Experience:
The description '(inferred)' displays in the editor.

Actual Experience:
The description '(inferred)' doesn't display in the editor.

More Info:

The description '(inferred)' displays after refreshing manually.
This issue doesn't reproduce when AzCopy is checked.

Won't fix since "This issue doesn't reproduce when AzCopy is checked."


Company form

Re-styled form and have added a way to add services with their specialties and sub-specialties
Type of change
Please delete options that are not relevant.

[X] New feature (non-breaking change which adds functionality)

Change Status

[X] Complete, but not tested (may need new tests)

Looks perfect


Empty tag deserialization

Hello,
I am having following problem with deserialization.
When I try to deserialize empty element like this, it works:
<LastName></LastName>
but when the empty element look like this, it does not work:
<LastName/>
This is how I create serializer instance:
var serializer = new ConfigurationContainer().EnableImplicitTyping(typeof(SampleModel)).Create();
Is there any way how to configure serializer in order to make it work? Or is it a bug?
Here is demo repository with sample data, just run project "RunMe": Github link
Thank you for help in advance.

Hello @hostar, thank you for writing in with this.  This appears to be an issue with the use of XmlTextReader. If I use XmlReader.Create(stream) this seems to work as expected.  I have provided a passing test of this for your review here:
https://github.com/ExtendedXmlSerializer/home/blob/ff303d4e991fab007fa245a119975d3abb7c373d/test/ExtendedXmlSerializer.Tests.ReportedIssues/Issue445Tests.cs#L13-L114
While we do provide compatibility for classic readers, our APIs really expect and support the use of XmlReader.Create, which is the recommended/traditional way of creating a reader based on input (stream).  Since XmlReader is an abstract class and there can be any number of implementations, ensuring 100% support for each and every one is unfortunately not a supported scenario.  That stated, if there is a bug somewhere in how we are handling the reading with XmlTextReader, I will gladly accept a PR to review and fix it.
Please let me know if you have any further questions/issues/concerns around this and I will do my best to assist you. 👍

Thank you very much, that was it. It did not even occur to me that could be a problem.
The specific reason for this behavior is the XmlTextReader is returning CanReadValueChunk as false in case of empty element:
https://github.com/ExtendedXmlSerializer/home/blob/ec86d89cdd2e03c1fd6ee5cbef60e5420c00550e/src/ExtendedXmlSerializer/ExtensionModel/Xml/XmlReader.cs#L50
Even though I would be probably able to fix it somehow, as I look how advanced your code is, my fix really would not be in accordance with the rest of project.
Therefore I consider this solved.

Oh wow weird.  Yeah, it is saying in the comments of XmlTextReader.CanReadValueChunk that it is unsupported.  Probably not a robust solution, but you could override XmlTextReader and force it to be true.  I provided an example test of this for you here:
https://github.com/ExtendedXmlSerializer/home/blob/23186442fdc12245384779648cf1da6825b7f17b/test/ExtendedXmlSerializer.Tests.ReportedIssues/Issue445Tests.cs#L51-L94
In any case, glad we got you pointed in a good direction. 👍


Enhancement/hint when unauthenticated

Overview
Fixes #974
After running more than 8 queries when unauthenticated, the coachmark below appears

Hovering over the coachmark displays this bubble content:

Notes
Optional. Ancillary topics, caveats, alternative strategies that didn't work out, anything else.
Testing Instructions
Sign out of GE.
Run 9 sample queries
A coachmark appears which prompts the user to sign in to see real data

Hey , could you update the testing instructions...
I was confused at the "Click on any sample queries other than GET" step...
Then I realised you meant that we should click on the GET request more than 5 times

Sure,
I’ve made the update
Sent from my iPhone

On 19 Nov 2021, at 12:54, Elinor @.***> wrote:

Hey , could you update the testing instructions...
I was confused at the "Click on any sample queries other than GET" step...
Then I realised you meant that we should click on the GET request more than 5 times
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

@Onokaev also note that this issue was resolved by this change but maybe we forgot to close it then. This change was added around two days after the issue was created.

@Onokaev @adhiambovivian  It is possible that this issue was solved by this change because it made it clear that a users need to sign in to access their data.
Before the change

After the change

@Onokaev @adhiambovivian It is possible that this issue was solved by this change because it made it clear that users need to sign in to access their data.
Before the change
After the change

Hi @jobala. Thanks for pointing this out. Yes, this change you've highlighted does address the issue. I am closing this PR


⚠ 한경주요지수-코스피200 has degraded performance

In bd9eb68, 한경주요지수-코스피200 (https://markets.hankyung.com/indices/kospi200) experienced degraded performance:

HTTP code: 200
Response time: 6885 ms

Resolved: 신규상장 performance has improved in a5ecf07.


Reduce more allocations in CreateFileMatcher

This includes https://github.com/Microsoft/msbuild/pull/2590, so be sure to only review the last commit https://github.com/Microsoft/msbuild/commit/16be8a35614dabf16c4e1e15c9f53ae14b7d368f.
This removes the capture class from CreateFileMatcher which was 0.3% of allocations (74 MB) in a large partner's solution:

I ran numbers, but I'm unsure if I trust the wins unless Picasso tends to hit this path alot:
DotnetConsoleProject | Evaluation: Time (ms) | ::ok_hand: | no | 29.0653 -> 29.056 (-0.032%)
DotnetConsoleProject | Evaluation: Memory | ::ok_hand: | no | 4578282 -> 4577477 (-0.018%)
DotnetWebProject | Evaluation: Time (ms) | :ok_hand: | no | 42.7263 -> 42.9768 (0.586%)
DotnetWebProject | Evaluation: Memory | ::ok_hand: | no | 5989377 -> 5988665 (-0.012%)
DotnetMvcProject | Evaluation: Time (ms) | ::ok_hand: | no | 79.5809 -> 79.3658 (-0.27%)
DotnetMvcProject | Evaluation: Memory | ::ok_hand: | no | 8178370 -> 8177419 (-0.012%)
Picasso | Evaluation: Time (ms) | :white_check_mark: | yes | 236.8097 -> 231.4155 (-2.278%)
Picasso | Evaluation: Memory | :white_check_mark: | yes | 31315772 -> 31250879 (-0.207%)
SmallP2POldCsproj | Evaluation: Time (ms) | :white_check_mark: | yes | 39.3329 -> 38.3375 (-2.531%)
SmallP2POldCsproj | Evaluation: Memory | :white_check_mark: | yes | 5071628 -> 5062990 (-0.17%)
Generated_100_100_v150 | Evaluation: Time (ms) | :white_check_mark: | yes | 1274.1377 -> 1266.6987 (-0.584%)
Generated_100_100_v150 | Evaluation: Memory | :white_check_mark: | yes | 165902878 -> 165260010 (-0.387%)
LargeP2POldCsproj | Evaluation: Time (ms) | :white_check_mark: | yes | 694.3973 -> 687.7223 (-0.961%)
LargeP2POldCsproj | Evaluation: Memory | :white_check_mark: | yes | 67646893 -> 67493319 (-0.227%)
roslyn | Evaluation: Time (ms) | :ok_hand: | no | 5107.4407 -> 5122.73 (0.299%)
roslyn | Evaluation: Memory | ::ok_hand: | no | 490482576 -> 487421657 (-0.624%)

tag @pilchie


client versions listing

Hey there!
I've just noticed the client version listing isn't complete yet:
powershift client versions

v1.3.2
v1.3.3
v1.4.0
v1.4.1
v1.5.0
v1.5.1
v3.6.0-alpha.1

However alpha 2 has also been already released:
https://github.com/openshift/origin/releases/tag/v3.6.0-alpha.2
Could you please point me to the place where the possible client versions are fetched from to make it possible to install alpha 2?
Thanks & Regards,
Fixer

They are fetched based on filenames listed on:

https://github.com/openshift/origin/releases

The package has already been updated though.
If used pipsi to install it, to upgrade run:
pipsi upgrade powershift-cli[all]

of if you used pip, use:
pip install -U "powershift-cli>1.1.16"

You can then do:
powershift client install v3.6.0-alpha.2

Just be aware that looks like they may have changed something in oc cluster up as it relates to MacOS X as it now longer works there and I may have to tweak how powershift cluster works. I don't know if this affects Windows as well.

May actually be working. Issue may have been caused by slow pull down of haproxy router image in background. The alpha 2 image is much larger than alpha 1, now 1.0GB in size, rather than 650MB as before. I only have slow Internet.

Thank you very much, that was a really-really fast resolve ! 🥇

I had had it on my TODO list for a few days before you raised it and it is a quick change. :-)


Add #traits_for_enum to RuboCop::RSpec::FactoryBot.reserved_methods

Thank you for building and maintaining rubocop-rspec. My team uses this and a lot of the defaults are pretty sensible, and help us to write cleaner tests!
FactoryBot recently released version 6.0.0, which introduces a new method to use within a factory definition, #traits_for_enum. Using this causes Rubocop to flag it with FactoryBot/AttributeDefinedStatically. This issue stems from the fact that this new method is not included in Rubocop::RSpec::FactoryBot.reserved_methods.
https://github.com/rubocop-hq/rubocop-rspec/blob/fad338729759334cb7411802e54cdf84f0e685c4/lib/rubocop/rspec/factory_bot.rb#L37-L40
A simple fix would be to include this reserved method in the list. I don't mind creating a PR for this.
Curious to hear your thoughts!

Hey! Thanks for bringing this up.
Sure, a pull request is welcome.


TestExternalDetailsService failure

I0214 02:11:54.269] --- FAIL: TestExternalDetailsService (68.65s)
I0214 02:11:54.270] 	demo_test.go:143: could not find<PHONE_NUMBER> in response
I0214 02:11:54.270] FAIL
https://k8s-gubernator.appspot.com/build/istio-prow/pull/istio_istio/3475/e2e-smoke/2952/

@ldemailly @andraxylia @rshriram @mandarjog we should decide how to treat failures to external sites, like google.com, googleapias.com, httpbin.org, wikipedia.org. The connections to these sites fail from time to time. Should we retry the test several times before failing?

Closing the issue, the tests to external services will fail from time to time.


Changed fail to fall.

I believe it should read 'fall back' and not 'fail back'.

@namanan : Thanks for your contribution! The author(s) have been notified to review your proposed change.

A Google search shows that "fail back" might be correct, but we'll ask the author @rayne-wiselman to take a look at the pull request. Thanks.

Current text is correct #please-close


[Monitoring V2 UI] Add RKE2 as a Cluster Type

This issue specifies an update to the Monitoring V2 UI Basic Configuration from https://github.com/rancher/dashboard/issues/908 and Monitoring V2 UI Advanced Configuration (https://github.com/rancher/dashboard/issues/909)
Basic Configuration (https://github.com/rancher/dashboard/issues/908)
General
Add an field under the Cluster Type field for RKE2
If Cluster Type is RKE2, rke2ControllerManager.enabled, rke2Scheduler.enabled, rke2Proxy.enabled,  and rke2Etcd.enabled should all be true by default.
Advanced Configuration (https://github.com/rancher/dashboard/issues/909)
Exporters
Export Kube Scheduler Metrics, Export Kube Controller Manager Metrics, Export Kube Proxy Metrics, and Export Etcd Metrics will need to also update the k8s YAML logic to also be able to point to rke2 fields (described above) based on the Cluster Type.
Note for QA: We should test whether an RKE2 cluster uses the right exporters and is able to scrape metrics as expected after this change is merged. The targets specified should both show up on the Prometheus UI and #/# targets and the default Grafana Dashboards for these components should also be working as expected.

[x] Backend Changes: https://github.com/rancher/charts/pull/544

Basic configuration is in review.

There will be one small change to this. The cluster type is still RKE2 but the label will display RKE Federal (see here)

The monitoring app is installed into the imported RKE2 cluster successfully. All parts are working as expected.
Rancher version: master-e919796c808d0aa643cb3491581bffcd64cfbe56-head, HA installation
RKE2 version: v1.18.8-beta19+rke2


What font is that?

https://github.com/classabbyamp/motdd/blob/main/doc/basic-example.png
What's the font in this screenshot? It looks nice.

https://input.djr.com


adds multiple adapters to a peft model

Draft PR to add multiple adapters to a base model.
Implemented for 8bit layers at the moment, I will attempt the implementation for merged layers tomorrow.
See examples/multiple_adapters.py for a very brief demo.
Early feedback would be great! cc @pacman100

I have updated the PR to use Tuples for r, dropout and alpha.
I was thinking that a better API for the user could be something like the following:
lora_config = LoraConfig(task_type="CAUSAL_LM")

model = prepare_model_for_int8_training(model)
model = get_peft_model(model, lora_config)

adaptor_config_a = AdapterConfig(r=8, target_modules=["q_proj", "v_proj"], ...)
model.add_adaptor("finetune_task_a", adapter_config_a)
model.enable_adapter("finetune_task_a")
# train on task a

adaptor_config_b = AdapterConfig(r=16, target_modules="a regex", ...)
model.add_adaptor("finetune_task_b", adapter_config_b)
model.enable_adapter("finetune_task_b")
# train on task b

Ths should resolve point a. The adapter layers could contain ModuleDicts rather than ModuleLists to make it clearer to the user which adapter they are enabling. All the _find_and_replace() logic would take place during the add_adapter() call.
This is a larger change. Beyond the scope of this PR, what do you think?
As for this PR, the next step would be to update the MergedLinear layers. Let me know what you think of the PR so far and then I can work on that.

I have updated the PR to use Tuples for r, dropout and alpha.

Hello, Thank you for this change. However, this isn't backward compatible and would be a breaking change. What I envisioned was that r, dropout and alpha would be Union[Tuple[dtype], dtype] where dtype is int/float. In post_init of the config, when a tuple is passed the length should be equal to the n_adapters value else a tuple is created with that length self.r = tuple([self.r]*self.n_adapters).
Please let me know your thoughts?
Also, the change you are suggesting to enable (a) is bigger change and indeed a better way, I'll look into it

Good point, I have updated the PR.

Regarding the larger change I was suggesting, I don't think it is a lot of work, but it would probably be a breaking change.

@pacman100 , I have updated the PR to include multiple adapted support for the other merged layers etc.
I started making some tests and I noticed that all the original tests fail on my machine, do they pass on yours? Is there CI workflow set up for this repo yet, if not I can create a PR for it?

Hello @edbeeching, thank you for the recent changes. I'll check the current tests tomorrow and fix the failing ones.
It would be great if you could raise PR for the CI integration 🤗

Would be great to be able to train adapters independently as usual and to load them together, with ability to quickly switch between them.

closing as superceded by #263


[stable/redis-ha]:/readonly-config/init.sh: line 84: Could not resolve the announce ip for this pod: not found

Encounter error with [stable/redis-ha]，detail logs show in config-init pod as :

Rancher: v2.2.5
Kubernetes: v1.14.1
Network: Canal v3.4.0  (with project network isolated disabled)
chart: stable/redis-ha (with redis:5.0.5-alpine)

With newly clone charts, I only change the pv size to 2Gi in values.yaml。
The following are details:
[root@k8s-master01 ~]# kubectl describe pod redis-cluster-redis-ha-server-0 -n redis-ns
Name:               redis-cluster-redis-ha-server-0
Namespace:          redis-ns
Priority:           0
PriorityClassName:
Node:               k8s-master03/<IP_ADDRESS>
Start Time:         Fri, 06 Sep 2019 12:30:47 +0800
Labels:             app=redis-ha
controller-revision-hash=redis-cluster-redis-ha-server-cdbf449f9
release=redis-cluster
statefulset.kubernetes.io/pod-name=redis-cluster-redis-ha-server-0
Annotations:        checksum/init-config: c99d75b4c87b504dd21ea68f78230a354bcac24c4a9bdb88c8eb0d25e150faad
cni.projectcalico.org/podIP: <IP_ADDRESS>/32
Status:             Pending
IP:                 <IP_ADDRESS>
Controlled By:      StatefulSet/redis-cluster-redis-ha-server
Init Containers:
config-init:
Container ID:  docker://d3a5f34505136b9f70eb286b54229f8f7ec9a33464c22d67e3608021033bd6ed
Image:         redis:5.0.5-alpine
Image ID:      docker-pullable://redis@sha256:50899ea1ceed33fa03232f3ac57578a424faa1742c1ac9c7a7bdb95cdf19b858
Port:
Host Port:
Command:
sh
Args:
/readonly-config/init.sh
State:          Waiting
Reason:       CrashLoopBackOff
Last State:     Terminated
Reason:       Error
Exit Code:    127
Started:      Fri, 06 Sep 2019 12:55:30 +0800
Finished:     Fri, 06 Sep 2019 12:55:50 +0800
Ready:          False
Restart Count:  9
Environment:
SENTINEL_ID_0:  bb40c01406652e900099be280304149f703d443a
SENTINEL_ID_1:  b9b0231bb959f53f2dfaa80343c4528e53c42d1c
SENTINEL_ID_2:  ad93c911889f67ba4cffb5ab91ea6cb2bf4417af
Mounts:
/data from data (rw)
/readonly-config from config (ro)
/var/run/secrets/kubernetes.io/serviceaccount from redis-cluster-redis-ha-token-gs46c (ro)
Containers:
redis:
Container ID:
Image:         redis:5.0.5-alpine
Image ID:
Port:          6379/TCP
Host Port:     0/TCP
Command:
redis-server
Args:
/data/conf/redis.conf
State:          Waiting
Reason:       PodInitializing
Ready:          False
Restart Count:  0
Liveness:       tcp-socket :6379 delay=15s timeout=1s period=10s #success=1 #failure=3
Environment:
Mounts:
/data from data (rw)
/var/run/secrets/kubernetes.io/serviceaccount from redis-cluster-redis-ha-token-gs46c (ro)
sentinel:
Container ID:
Image:         redis:5.0.5-alpine
Image ID:
Port:          26379/TCP
Host Port:     0/TCP
Command:
redis-sentinel
Args:
/data/conf/sentinel.conf
State:          Waiting
Reason:       PodInitializing
Ready:          False
Restart Count:  0
Liveness:       tcp-socket :26379 delay=15s timeout=1s period=10s #success=1 #failure=3
Environment:
Mounts:
/data from data (rw)
/var/run/secrets/kubernetes.io/serviceaccount from redis-cluster-redis-ha-token-gs46c (ro)
Conditions:
Type              Status
Initialized       False
Ready             False
ContainersReady   False
PodScheduled      True
Volumes:
data:
Type:       PersistentVolumeClaim (a reference to a PersistentVolumeClaim in the same namespace)
ClaimName:  data-redis-cluster-redis-ha-server-0
ReadOnly:   false
config:
Type:      ConfigMap (a volume populated by a ConfigMap)
Name:      redis-cluster-redis-ha-configmap
Optional:  false
redis-cluster-redis-ha-token-gs46c:
Type:        Secret (a volume populated by a Secret)
SecretName:  redis-cluster-redis-ha-token-gs46c
Optional:    false
QoS Class:       BestEffort
Node-Selectors:
Tolerations:     node.kubernetes.io/not-ready:NoExecute for 300s
node.kubernetes.io/unreachable:NoExecute for 300s
Events:
Type     Reason            Age                   From                   Message

Warning  FailedScheduling  27m (x4 over 28m)     default-scheduler      pod has unbound immediate PersistentVolumeClaims (repeated 2 times)
Normal   Scheduled         27m                   default-scheduler      Successfully assigned redis-ns/redis-cluster-redis-ha-server-0 to k8s-master03
Normal   Pulling           27m                   kubelet, k8s-master03  Pulling image "redis:5.0.5-alpine"
Normal   Pulled            27m                   kubelet, k8s-master03  Successfully pulled image "redis:5.0.5-alpine"
Normal   Created           24m (x5 over 27m)     kubelet, k8s-master03  Created container config-init
Normal   Started           24m (x5 over 27m)     kubelet, k8s-master03  Started container config-init
Normal   Pulled            22m (x5 over 26m)     kubelet, k8s-master03  Container image "redis:5.0.5-alpine" already present on machine
Warning  BackOff           2m25s (x99 over 26m)  kubelet, k8s-master03  Back-off restarting failed container
====================
[root@k8s-master01 ~]# kubectl logs -f redis-cluster-redis-ha-server-0 -c config-init -n redis-ns
Could not connect to Redis at redis-cluster-redis-ha:26379: Try again
Initializing config..
/readonly-config/init.sh: line 84: Could not resolve the announce ip for this pod: not found
[root@k8s-master01 ~]#

I have one master node with two worker node, showing as the picture:

Anyone can help ?
Thanks in advance.

@Water0 Do you have the services deployed successfully with bound IP addresses?
The init container will try to resolve the service IP here.
ANNOUNCE_IP=$(getent hosts "$SERVICE-announce-$INDEX" | awk '{ print $1 }')

@Water0 Do you have the services deployed successfully with bound IP addresses?
The init container will try to resolve the service IP here.
ANNOUNCE_IP=$(getent hosts "$SERVICE-announce-$INDEX" | awk '{ print $1 }')

I can get the annouce_ip in another running pod with full FQDN name, show as the following pics:

@DandyDeveloper
Thanks for you reply! I can get the annouce_ip in another running pod with full FQDN name. All of the announce service are type of ClusterIP while the other is of Headless.

@Water0 Are you running on-prem? Do you have hairpin mode enabled for the networking plugin you have enabled?
Typically, pods cannot communicate with their own services and vice-versa unless you have the hairpin mode enabled for your K8 networking plugin.
That's the only logic reason I can think you'd be having this problem.

I am running into this issue as well, when i tried setting up HPA on redis-ha stateful set , the new pods run into this problem
│ /readonly-config/init.sh: line 84: Could not resolve the announce ip for this pod: not found

@kish3007, sorry for no updates since I'm busy with other tasks.  Per the reply by @DandyDeveloper,  incorrect network maybe the cause. Following the link:
https://kubernetes.io/docs/tasks/debug-application-cluster/debug-service/#a-pod-cannot-reach-itself-via-service-ip
I will be back to double check & try it later.  Thanks @DandyDeveloper for your help!

@Water0 i dont think there is a problem with network plugin, its just that we are not able to scale the statefulset redis pods after the helm chart is deployed with replica count, i have a hpa.yaml like below:
kind: HorizontalPodAutoscaler
metadata:
  name: redis-hpa
  namespace: redis
spec:
  maxReplicas: {{ .Values.autoscaling.max_replicas }}
  minReplicas: {{ .Values.autoscaling.min_replicas }}
  scaleTargetRef:
    apiVersion: apps/v1
    kind: StatefulSet
    name: redis-redis-ha-server
  targetCPUUtilizationPercentage: 80

This didnt work as the stateful set redis pods have dependency to init process and also it needs redis-redis-ha-announce service to be up before we scale the actual redis pods, so thats why it needs a feature request to support HPA as part of the helm chart coz right now someone need to manually scale up by overriding the replicaCount in helm upgrade process.

I have the same problem with a new deployment on empty kubernetes cluster 1.16. I followed link provided by @Water0 and I have hairpin enabled, so networking probably is not the case. I tried nslookup'ing of service and it's not in DNS.

Still have this issue was it ever found the cause to be network CNI?

I am also running into this issue. Anyone has had any luck fixing this?

Any updates? I have the same problem with brand new kubernetes v1.17.9 (kubeproxy in ipvs mode), and redis-ha helm version 3.9.2.


Use -static for full static compilation

This allows the use of tini within even more minimal environments (such as images that are FROM scratch with a single application binary COPY'd in).
This is just a proposal, so I'm definitely willing to debate and/or update and/or close, but I think this is worth discussing. :smile:

Hey @tianon; thanks for the PR!
I originally had static compilation in Tini, but took it out because it was breaking clang builds and I didn't feel it was useful enough to bother finding out what the problem was (https://github.com/krallin/tini/commit/699c3aadf25944975a390fa1bd333eb1ef4b5d89). You fixing it takes that out of the equation, so I'm all for having a static build. : )
Now, I'd rather have two build targets (a static one and a dynamic one), rather than static-only. Any reason not to? Let me know what you think.

Oh, and just to be clear: when I say "I'd rather have two", I'm not asking you to do it. I can use your code and add some of the old one I linked earlier to do so.
(though obviously, if you want to do it, I'll merge it! :) )
Cheers,

That sounds fantastic!  I just want to make sure I can use your published binaries instead of compiling my own. :heart:
I'll openly admit that my CMake-fu is not nearly strong enough for me to easily adapt this to have two targets, so feel free to steal all my work to make this happen how you want it done. :+1:

Oh, looking at that commit, I think I could hammer this out.  Give me a little time and I'll punt again if I can't get it done. :+1:

I see you updated it, thanks!
I'm going to check whether the images build properly and merge this in if so (I might not have time to do it tonight but if so I'll get to it tomorrow).
Thanks again!

I think I got everything, but obviously feel free to comment if anything looks out of place in what I've done. :smile:
(a few places have changed a bit since that older commit)

Thanks for saving me from having to create this utility myself. :+1:

Awesome, sounds good!  I'm in no rush (my current use cases are fine with non-static), so take the time you need. :heart:

I had to make a few changes (basically updating the template files instead of the resulting files, and fixing the valgrind test which doesn't like static binaries).
I'm going to push those as a separate PR: #4. (I don't think I can make changes to this PR, but I made sure to retain the author).
Thanks again!


Unsatisfied version of OpenCL found for versions earlier than 2.1

Describe the bug
The current version of TornadoVM requires OpenCL 2.1 version to be installed since it utilizes in the native layer the clCreateProgramWithIL function. This function has been added since this commit and it is used to pass SPIR-V binaries and execute them through OpenCL.
However, if TornadoVM is built without OpenCL 2.1 and it is configured to be built with only the OpenCL backend, this function is not used but it is looked if is exist in the shared library of OpenCL (libOpenCL.so).
And since it is mising, it throws the following error:
$ tornado --devices
.lang.UnsatisfiedLinkError: version `OPENCL_2.1' not found (required by /home/thanos/repositories/TornadoVM/dist/tornado-sdk/tornado-sdk-0.14-dev-700f839/lib/libtornado-opencl.so)
        at java.base/java.lang.ClassLoader$NativeLibrary.load0(Native Method)
        at java.base/java.lang.ClassLoader$NativeLibrary.load(ClassLoader.java:2445)
        at java.base/java.lang.ClassLoader$NativeLibrary.loadLibrary(ClassLoader.java:2501)
        at java.base/java.lang.ClassLoader.loadLibrary0(ClassLoader.java:2700)
        at java.base/java.lang.ClassLoader.loadLibrary(ClassLoader.java:2662)
        at java.base/java.lang.Runtime.loadLibrary0(Runtime.java:830)
        at java.base/java.lang.System.loadLibrary(System.java:1873)
        at tornado.drivers.opencl@0.14-dev/uk.ac.manchester.tornado.drivers.opencl.OpenCL.<clinit>(OpenCL.java:69)
        at tornado.drivers.opencl@0.14-dev/uk.ac.manchester.tornado.drivers.opencl.OCLDriver.<init>(OCLDriver.java:56)
        at tornado.drivers.opencl@0.14-dev/uk.ac.manchester.tornado.drivers.opencl.OCLTornadoDriverProvider.createDriver(OCLTornadoDriverProvider.java:50)
        at tornado.runtime@0.14-dev/uk.ac.manchester.tornado.runtime.TornadoCoreRuntime.loadDrivers(TornadoCoreRuntime.java:173)
        at tornado.runtime@0.14-dev/uk.ac.manchester.tornado.runtime.TornadoCoreRuntime.<init>(TornadoCoreRuntime.java:107)
        at tornado.runtime@0.14-dev/uk.ac.manchester.tornado.runtime.TornadoCoreRuntime.<clinit>(TornadoCoreRuntime.java:83)
        at tornado.drivers.common@0.14-dev/uk.ac.manchester.tornado.drivers.TornadoDeviceQuery.main(TornadoDeviceQuery.java:73)

How To Reproduce
You will need a machine that has not installed OpenCL 2.1 and you can run:
$ tornado --devices

Expected behavior
To be functional with OpenCL 1.2 when OpenCL is selected as a backend, and to receive a warning regarding the missing compatible version if SPIR-V is going to be configured for building.
Computing system setup (please complete the following information):

OS: Centos 7
OpenCL Version: 1.2
PTX Version (if applicable)
Level Zero & SPIRV Versions (if applicable)
TornadoVM commit id: 700f839

I took a look at this. So we import all Khronos OpenCL headers to facilitate the installation of the OpenCL backend. In these headers, we use OpenCL 3.0. The issue is that, as soon as the clCreateProgramWithIL, we need to load the so dynamic library with this function in it.
Since NVIDIA and Intel support OpenCL 3.0, I suggest that TornadoVM requires at least OpenCL 2.1. For the FPGA builds, we can have a custom-built to avoid this call (all major OpenCL implementations for OpenCL are based on OpenCL 1.0, still).


Lambda rollback stage implementation

What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?:

Failed lambda application deployment can be rolled back automatically

I was taking a walk around the implementation for lambda cloud provider, then I looked out for this part:
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L274-L275
Instead it'd better be:
	for version, weight := range cfg.RoutingConfig.AdditionalVersionWeights {
		newerVersionTraffic = percentageToPercent(aws.Float64Value(weight))

Also, as you may know all around here should be replaced with aws.StringValue().
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L266
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L282
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L194
I should've mentioned on previous review, but hopefully it helps you.

I was taking a walk around the implementation for lambda cloud provider, then I looked out for this part:
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L274-L275
Instead it'd better be:
	for version, weight := range cfg.RoutingConfig.AdditionalVersionWeights {
		newerVersionTraffic = percentageToPercent(aws.Float64Value(weight))

Also, as you may know all around here should be replaced with aws.StringValue().
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L266
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L282
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L194
I should've mentioned on previous review, but hopefully it helps you.

I was taking a walk around the implementation for lambda cloud provider, then I found out this part:
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L274-L275
Instead it'd better be:
	for version, weight := range cfg.RoutingConfig.AdditionalVersionWeights {
		newerVersionTraffic = percentageToPercent(aws.Float64Value(weight))

Also, as you may know all around here should be replaced with aws.StringValue().
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L266
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L282
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L194
I should've mentioned on previous review, but hopefully it helps you.

Thanks for your comment, I will make a separate PR for those fixes 😁

I was taking a walk around the implementation for lambda cloud provider, then I found out this part:
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L274-L275
Instead it'd better be:
	for version, weight := range cfg.RoutingConfig.AdditionalVersionWeights {
		newerVersionTraffic = percentageToPercent(aws.Float64Value(weight))

Also, as you may know all around here should be replaced with aws.StringValue().
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L266
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L282
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L194
I should've mentioned on previous review, but hopefully it helps you.

Thanks for your comment, I will make a separate PR for those fixes 😁

I was taking a walk around the implementation for lambda cloud provider, then I found out this part:
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L274-L275
Instead it'd better be:
	for version, weight := range cfg.RoutingConfig.AdditionalVersionWeights {
		newerVersionTraffic = percentageToPercent(aws.Float64Value(weight))

Also, as you may know all around here should be replaced with aws.StringValue().
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L266
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L282
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L194
I should've mentioned on previous review, but hopefully it helps you.

@nakabonne PTAL 😁
https://github.com/pipe-cd/pipe/pull/1464

I was taking a walk around the implementation for lambda cloud provider, then I found out this part:
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L274-L275
Instead it'd better be:
	for version, weight := range cfg.RoutingConfig.AdditionalVersionWeights {
		newerVersionTraffic = percentageToPercent(aws.Float64Value(weight))

Also, as you may know all around here should be replaced with aws.StringValue().
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L266
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L282
https://github.com/pipe-cd/pipe/blob/d4d222200a9fb5df5c558f9b13c3256e51f202de/pkg/app/piped/cloudprovider/lambda/client.go#L194
I should've mentioned on previous review, but hopefully it helps you.

@nakabonne PTAL 😁
https://github.com/pipe-cd/pipe/pull/1464

@nghialv @nakabonne PTAL 🙏

@nghialv @nakabonne PTAL 🙏

Thank you.
/lgtm

Thank you.
/lgtm

🚀
/approve

🚀
/approve


Update README.md

Move the "buy me a coffee button" from below Todo section to the top of Todo section.

Hey I can work on this. Looking for a good place to start to contribute to open source projects.

Welcome here, update the README.md and make a PR

You are welcome here, update the README.md and make a PR.

Hi @tawhidmonowar  I was thinking if it would make more sense to create a new headline like this one, to include on the button? What do you think?

I think it will be great.