build failure on upcoming gcc-12: test/src/unit-regression1.cpp:392:22: error: ambiguous overload for 'operator='

On gcc-12 (20211107 snapshot) std::string amended it's constructors to a form that fails regression tests building:
mkdir b && cd b
cmake ..
make
...
[ 71%] Building CXX object test/CMakeFiles/test-regression1.dir/src/unit-regression1.cpp.o
/home/slyfox/dev/git/nlohmann-json/test/src/unit-regression1.cpp: In function 'void _DOCTEST_ANON_FUNC_2()':
/home/slyfox/dev/git/nlohmann-json/test/src/unit-regression1.cpp:392:22: error: ambiguous overload for 'operator=' (operand types are 'std::string' {aka 'std::__cxx11::basic_string<char>'} and 'nlohmann::basic_json<>::value_type' {aka 'nlohmann::basic_json<>'})
  392 |         s2 = o["name"];
      |                      ^
In file included from /nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/string:53,
                 from /nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/bits/locale_classes.h:40,
                 from /nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/locale:39,
                 from /home/slyfox/dev/git/nlohmann-json/test/src/unit-regression1.cpp:33:
/nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/bits/basic_string.h:627:21: note: candidate: 'std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator=(std::nullptr_t) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>; std::nullptr_t = std::nullptr_t]' (deleted)
  627 |       basic_string& operator=(nullptr_t) = delete;
      |                     ^~~~~~~~
/nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/bits/basic_string.h:682:7: note: candidate: 'std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator=(const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]'
  682 |       operator=(const basic_string& __str)
      |       ^~~~~~~~
/nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/bits/basic_string.h:720:7: note: candidate: 'std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator=(std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]'
  720 |       operator=(basic_string&& __str)
      |       ^~~~~~~~
In file included from /home/slyfox/dev/git/nlohmann-json/test/thirdparty/doctest/doctest_compatibility.h:6,
                 from /home/slyfox/dev/git/nlohmann-json/test/src/unit-regression1.cpp:30:
/home/slyfox/dev/git/nlohmann-json/test/src/unit-regression1.cpp:398:37: error: ambiguous overload for 'operator=' (operand types are 'std::string' {aka 'std::__cxx11::basic_string<char>'} and 'nlohmann::basic_json<>::value_type' {aka 'nlohmann::basic_json<>'})
  398 |         CHECK_THROWS_AS(s2 = o["int"], json::type_error);
      |                                     ^
/home/slyfox/dev/git/nlohmann-json/test/thirdparty/doctest/doctest.h:1844:23: note: in definition of macro 'DOCTEST_CAST_TO_VOID'
 1844 |     static_cast<void>(__VA_ARGS__);                                                                \
      |                       ^~~~~~~~~~~
/home/slyfox/dev/git/nlohmann-json/test/thirdparty/doctest/doctest.h:2163:44: note: in expansion of macro 'DOCTEST_ASSERT_THROWS_AS'
 2163 | #define DOCTEST_CHECK_THROWS_AS(expr, ...) DOCTEST_ASSERT_THROWS_AS(expr, DT_CHECK_THROWS_AS, "", __VA_ARGS__)
      |                                            ^~~~~~~~~~~~~~~~~~~~~~~~
/home/slyfox/dev/git/nlohmann-json/test/thirdparty/doctest/doctest.h:2576:36: note: in expansion of macro 'DOCTEST_CHECK_THROWS_AS'
 2576 | #define CHECK_THROWS_AS(expr, ...) DOCTEST_CHECK_THROWS_AS(expr, __VA_ARGS__)
      |                                    ^~~~~~~~~~~~~~~~~~~~~~~
/home/slyfox/dev/git/nlohmann-json/test/src/unit-regression1.cpp:398:9: note: in expansion of macro 'CHECK_THROWS_AS'
  398 |         CHECK_THROWS_AS(s2 = o["int"], json::type_error);
      |         ^~~~~~~~~~~~~~~
In file included from /nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/string:53,
                 from /nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/bits/locale_classes.h:40,
                 from /nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/locale:39,
                 from /home/slyfox/dev/git/nlohmann-json/test/src/unit-regression1.cpp:33:
/nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/bits/basic_string.h:627:21: note: candidate: 'std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator=(std::nullptr_t) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>; std::nullptr_t = std::nullptr_t]' (deleted)
  627 |       basic_string& operator=(nullptr_t) = delete;
      |                     ^~~~~~~~
/nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/bits/basic_string.h:682:7: note: candidate: 'std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator=(const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]'
  682 |       operator=(const basic_string& __str)
      |       ^~~~~~~~
/nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/bits/basic_string.h:720:7: note: candidate: 'std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator=(std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]'
  720 |       operator=(basic_string&& __str)
      |       ^~~~~~~~
In file included from /home/slyfox/dev/git/nlohmann-json/test/thirdparty/doctest/doctest_compatibility.h:6,
                 from /home/slyfox/dev/git/nlohmann-json/test/src/unit-regression1.cpp:30:
/home/slyfox/dev/git/nlohmann-json/test/src/unit-regression1.cpp:402:39: error: ambiguous overload for 'operator=' (operand types are 'std::string' {aka 'std::__cxx11::basic_string<char>'} and 'nlohmann::basic_json<>::value_type' {aka 'nlohmann::basic_json<>'})
  402 |         CHECK_THROWS_WITH(s2 = o["int"], "[json.exception.type_error.302] type must be string, but is number");
      |                                       ^
/home/slyfox/dev/git/nlohmann-json/test/thirdparty/doctest/doctest.h:1844:23: note: in definition of macro 'DOCTEST_CAST_TO_VOID'
 1844 |     static_cast<void>(__VA_ARGS__);                                                                \
      |                       ^~~~~~~~~~~
/home/slyfox/dev/git/nlohmann-json/test/thirdparty/doctest/doctest.h:2167:46: note: in expansion of macro 'DOCTEST_ASSERT_THROWS_WITH'
 2167 | #define DOCTEST_CHECK_THROWS_WITH(expr, ...) DOCTEST_ASSERT_THROWS_WITH(expr, #expr, DT_CHECK_THROWS_WITH, __VA_ARGS__)
      |                                              ^~~~~~~~~~~~~~~~~~~~~~~~~~
/home/slyfox/dev/git/nlohmann-json/test/thirdparty/doctest/doctest.h:2577:38: note: in expansion of macro 'DOCTEST_CHECK_THROWS_WITH'
 2577 | #define CHECK_THROWS_WITH(expr, ...) DOCTEST_CHECK_THROWS_WITH(expr, __VA_ARGS__)
      |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~
/home/slyfox/dev/git/nlohmann-json/test/src/unit-regression1.cpp:402:9: note: in expansion of macro 'CHECK_THROWS_WITH'
  402 |         CHECK_THROWS_WITH(s2 = o["int"], "[json.exception.type_error.302] type must be string, but is number");
      |         ^~~~~~~~~~~~~~~~~
In file included from /nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/string:53,
                 from /nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/bits/locale_classes.h:40,
                 from /nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/locale:39,
                 from /home/slyfox/dev/git/nlohmann-json/test/src/unit-regression1.cpp:33:
/nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/bits/basic_string.h:627:21: note: candidate: 'std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator=(std::nullptr_t) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>; std::nullptr_t = std::nullptr_t]' (deleted)
  627 |       basic_string& operator=(nullptr_t) = delete;
      |                     ^~~~~~~~
/nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/bits/basic_string.h:682:7: note: candidate: 'std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator=(const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]'
  682 |       operator=(const basic_string& __str)
      |       ^~~~~~~~
/nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/bits/basic_string.h:720:7: note: candidate: 'std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator=(std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]'
  720 |       operator=(basic_string&& __str)
      |       ^~~~~~~~
/home/slyfox/dev/git/nlohmann-json/test/src/unit-regression1.cpp:1174:16: error: ambiguous overload for 'operator=' (operand types are 'std::string' {aka 'std::__cxx11::basic_string<char>'} and 'nlohmann::json' {aka 'nlohmann::basic_json<>'})
 1174 |         test = v;
      |                ^
In file included from /nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/string:53,
                 from /nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/bits/locale_classes.h:40,
                 from /nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/locale:39,
                 from /home/slyfox/dev/git/nlohmann-json/test/src/unit-regression1.cpp:33:
/nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/bits/basic_string.h:627:21: note: candidate: 'std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator=(std::nullptr_t) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>; std::nullptr_t = std::nullptr_t]' (deleted)
  627 |       basic_string& operator=(nullptr_t) = delete;
      |                     ^~~~~~~~
/nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/bits/basic_string.h:682:7: note: candidate: 'std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator=(const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]'
  682 |       operator=(const basic_string& __str)
      |       ^~~~~~~~
/nix/store/0hvbmcf38gqpr6c49h7zl3iy228afaz0-gcc-12.0.0/include/c++/12.0.0/bits/basic_string.h:720:7: note: candidate: 'std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator=(std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]'
  720 |       operator=(basic_string&& __str)
      |       ^~~~~~~~
make[2]: *** [test/CMakeFiles/test-regression1.dir/build.make:76: test/CMakeFiles/test-regression1.dir/src/unit-regression1.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:2148: test/CMakeFiles/test-regression1.dir/all] Error 2
make: *** [Makefile:146: all] Error 2

Which compiler and operating system are you using?

Compiler: gcc-12 development snapshot
Operating system: x86_64-linux

Which version of the library did you use?

[x] the develop branch

I would like to test this in the CI - can you tell me how I can get a development snapshot of GCC, preferably for Ubuntu?

Oh, that's a good question. Looking around ubuntu has https://launchpad.net/ubuntu/+source/gcc-snapshot. Looking at impish release latest release there is 20210827. It does not yet have a std::string change added on Oct 1 in https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=cf876562c592193732f869e9f96034a42d0fad89 . I hope it eventually catches up.
From superficial test in VM it's enough to install it as:
$ apt install gcc-snapshot
$ export PATH=/usr/lib/gcc-snapshot/bin:$PATH
$ cmake ...

I personally build gcc locally from https://ftp.mpi-inf.mpg.de/mirrors/gnu/mirror/gcc.gnu.org/pub/gcc/snapshots/LATEST-12/ (Current is from Oct 14).

Thanks, I was not aware of package gcc-snapshot.

I can reproduce the issue with gcc-latest (GCC) 12.0.0 20211114 (experimental).

The failing regression tests are for #144 and #464.

Same error occurred again in #3226

GCC 12 is now in Fedora Rawhide, if that helps with testing.
(I found this bug because it’s causing https://github.com/monocasual/giada builds to fail in Fedora Rawhide.)

I can confirm. All dependent packages are now FTBFS.

That's too bad. I have not yet understood the reason for the error, let alone whether it's a bug in the library and how to fix it.

As described in https://en.cppreference.com/w/cpp/23, we will have the same problem with Clang libc++ 13 and MSVC STL 19.30. What I have not understood is whether deleting basic_string(nullptr_t) is actually meant to also affect C++11/C++14/C++17 builds.

What I have not understood is whether deleting basic_string(nullptr_t) is actually meant to also affect C++11/C++14/C++17 builds.

I have found https://www.kitware.com/itk-informs-c-stl-development/ a while ago and from that I would say this is only deleted in C++23 builds. See also https://github.com/microsoft/STL/pull/1995#issuecomment-864328370 from linked from there.

The deleted basic_string(nullptr) constructor is new in C++23, but constructing a std::string from nullptr was always undefined behaviour, so I made it present unconditionally for C++11/14/17/20 in GCC's std::lib. I can't understand what the conversions on your code are supposed to be doing, so I can't tell whether your code was always doing something bad and it's only being diagnosed now, or if the new constructor changes the behaviour of correct code.
Could you give a simplified example of what conversion is supposed to happen when constructing a std::string from a basic_json<>?

I think this is probably an approximation of what you're doing, right?
#include <string>

struct S
{
  operator const char*() const { return ""; }
  operator std::nullptr_t() const { return {}; }
};

std::string s{ S{} };

This is valid in C++20 and earlier, but ill-formed in C++23. And it demonstrates that the change should be limited to C++23. I'll change GCC.

Filed as a GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104099

I've compiled latest gcc-trunk locally here g++ (GCC) 12.0.1 20220118 (experimental) and with the following patch
diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp
index 92308c557..8fd2b357f 100644
--- a/single_include/nlohmann/json.hpp
+++ b/single_include/nlohmann/json.hpp
@@ -19043,6 +19043,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
     template < typename ValueType, typename std::enable_if <
                    detail::conjunction <
                        detail::negation<std::is_pointer<ValueType>>,
+                       detail::negation<std::is_same<ValueType, nullptr_t>>,
                        detail::negation<std::is_same<ValueType, detail::json_ref<basic_json>>>,
                                         detail::negation<std::is_same<ValueType, typename string_t::value_type>>,
                                         detail::negation<detail::is_basic_json<ValueType>>,

it compiles and all tests pass. The fix goes in the same direction as https://github.com/nlohmann/json/issues/3138#issuecomment-1015551459. It would have helped me if I read the reply first before trying to fix it ;)

Thanks @t-b for the fix! I tried it, and it worked for GCC 12. Unfortunately, it does not work for any other compiler :-/
See https://github.com/nlohmann/json/pull/3226.
The error is
/__w/json/json/single_include/nlohmann/json.hpp:19046:65: error: template argument for template type parameter must be a type
                       detail::negation<std::is_same<ValueType, nullptr_t>>,
                                                                ^~~~~~~~~

Any idea?

Should be std::nullptr_t

Good catch! Totally overlooked that. :-/

The code compiles, but there is one test failing when compiling the code with C++20:
TEST CASE:  lexicographical comparison operators
  values
  comparison: less

/__w/json/json/test/src/unit-comparison.cpp:213: ERROR: CHECK( (j_values[i] < j_values[j]) == expected[i][j] ) is NOT correct!
  values: CHECK( false == 1 )
  logged: i := 12
          j := 13
          j_values[i] := [1,2,3]
          j_values[j] := ["one","two","three"]

/__w/json/json/test/src/unit-comparison.cpp:213: ERROR: CHECK( (j_values[i] < j_values[j]) == expected[i][j] ) is NOT correct!
  values: CHECK( true == 0 )
  logged: i := 13
          j := 12
          j_values[i] := ["one","two","three"]
          j_values[j] := [1,2,3]

The same test succeeds with Clang (Linux, macOS) in C++20 mode. Any ideas?

Looks like a change in how the default comparisons are happening in C++20 with the new generated comparisons.  Looking at this in Compiler Explorer to see if I can track it down.

Confirmed different behavior of comparing std::vector<nlohmann::json<>> between -std=c++17 and -std=c++20 in GCC trunk.

Would it make sense to implement operator<=> for basic_json instead of the usual operators when compiled for C++20?

If you have implicit conversions then yes, you should provide your own <=>. Otherwise std::lib components that want to use <=> will do so via your implicit conversions, with possibly unexpected results.

I dug into that and still couldn't get it to pass.  It would be good to replace all the comparison and equality operators with <=> for both the main class and the type enum, but it won't necessarily fix this issue.

I dug into that and still couldn't get it to pass. It would be good to replace all the comparison and equality operators with <=> for both the main class and the type enum, but it won't necessarily fix this issue.

Worse than "won't necessarily fix this issue", libcxx [1] doesn't fully implement three way comparison for various std types yet, creating more problems. I've tried.
So, we'll have to solve/work around the underlying issue using the existing operators first, then optionally add the spaceship operator and guard that with requires clauses to work around missing standard library support.
Guess I know how I'll spend the weekend. :wink:
[1] https://libcxx.llvm.org/Status/Spaceship.html


Fixes #362

voila :)

Note that you named your branch 'fb-bagpipe-impl' while you implements 'Cimbalom'.
Also remember to delete IDE automatic comment headers at beginning of your file.


Why do not video fast forward and retreat?

Current behavior
Describe what happens when you encounter this issue.
Why do not video fast forward and retreat?
Reproduction steps
A 1, 2, 3, etc. list of what's needed to see the issue happen.
Expected behavior
Describe what you wanted to happen
Video can be fast forward and fast back.
thanks
Platform
Which player are you experiencing the problem on:

iOS
Android ExoPlayer
Android MediaPlayer
Windows UWP
Windows WPF

Video sample
If possible, include a link to the video that has the problem that can be streamed or downloaded from.

I use seek and currentTime to achieve the function of fast forward and quick return.


Multi classes for board titles

In the documentation it says that a board header can have multiple classes added:
  {
       "id"    : "board-id-1",               // id of the board
       "title" : "Board Title",              // title of the board
       "class" : "class1,class2,...",        // css classes to add at the title

However, it you do include multiple classes in a comma-separated string, the are rendered a single long class with commas in the name, rather than a space-separated series of individual classes:

Backlog

I tested this one and couldn't reproduce the issue. Can you provide an example?

I couldn't reproduce the issue, closed.


Onnx granite

What does this PR do?
This PR adds support for models using IBM's GraniteForCausalLM architecture when converting to ONNX. The key changes are:

Allow users to opt into using transformers>=4.45 for onnx conversions No longer needed
Add "granite" to model configs and tasks
Add "granite" as a model_type that uses grouped attention

NOTE: I encountered an issue very similar to the one discussed in https://github.com/huggingface/optimum/issues/1835. The root cause for me was the need to add "granite" to the list of models requiring Grouped Query Attention in modeling_decoder.py. I don't believe that is the root cause for #1835 since "llama" is already present there, but it is likely a similar issue showing up in the inference module using num_attention_heads instead of num_key_value_heads.
Rationale
This PR specifically addresses the "GraniteForCausalLM" architecture for IBM's forthcoming Granite family of models. The current ibm/PowerLM-3b model use this architecture and can be used as a placeholder for testing until the new models are released. The one exception is that the PowerLM model has num_attention_heads and num_key_value_heads set to match (no Grouped Query Attention) whereas the new models will use that (thus the need for the change to ensure GQA is used for "granite" at inference time).
Testing
When testing locally, I had the following dependency versions:
onnx==1.16.2
onnxruntime==1.19.2
torch==2.4.1
torchvision==0.19.1
transformers==4.45.2

To test the conversion, I did the following:
optimum-cli export onnx \
  --model $HOME/models/powerlm-3b \
  $HOME/models/powerlm-3b-onnx \
  --task text-generation-with-past

To evaluate the output side-by-side with the source model, I used the following script:

side_by_side.py
"""
Simple function to run and time pre and post optimized models
"""

# Standard
from datetime import timedelta
import argparse
import os
import time

# Third Party
from optimum.onnxruntime import ORTModelForCausalLM
from transformers import AutoTokenizer, AutoModelForCausalLM
import torch

def maybe_to_device(inputs: dict[str, torch.Tensor], device: str | None):
    """Send inputs to the device if desired"""
    if device:
        for k, v in inputs.items():
            inputs[k] = v.to(device)

def run_and_time(
    label: str,
    model_path: str,
    model_class: ORTModelForCausalLM | AutoModelForCausalLM,
    prompt: str,
    device: str | None,
    **kwargs,
):
    start_time = time.time()
    tokenizer = AutoTokenizer.from_pretrained(model_path)
    model = model_class.from_pretrained(model_path, device_map=device)
    load_end_time = time.time()
    inputs = tokenizer(prompt, return_tensors="pt")

    #DEBUG
    breakpoint()

    tok_end_time = time.time()
    maybe_to_device(inputs, device)
    outputs = model.generate(**inputs, **kwargs)
    gen_end_time = time.time()
    res = tokenizer.decode(outputs[0])
    end_time = time.time()
    print(f"------ {label} ------")
    print(res)
    print(f"Total Time: {timedelta(seconds=end_time-start_time)}")
    print(f"Load Time: {timedelta(seconds=load_end_time-start_time)}")
    print(f"Generate Time: {timedelta(seconds=gen_end_time-tok_end_time)}")
    print()

# Defaults
home = os.getenv("HOME")
assert home, "Need $HOME!"
orig_model_path = f"{home}/models/PowerLM-3b"
onnx_model_path = f"{home}/models/PowerLM-3b-onnx-O4"
prompt = "Write a code to find the maximum value in a list of numbers."
device = "cuda" if torch.cuda.is_available() else None

def main():
    parser = argparse.ArgumentParser(description=__doc__)
    parser.add_argument("--prompt", "-p", default=prompt)
    parser.add_argument("--raw-model", "-r", type=str, default=None)
    parser.add_argument("--onnx-model", "-o", type=str, default=None)
    parser.add_argument("--device", "-d", type=str, default=device)
    args = parser.parse_args()
    if args.raw_model:
        run_and_time("Transformers", args.raw_model, AutoModelForCausalLM, args.prompt, args.device, max_new_tokens=100)
    if args.onnx_model:
        run_and_time("ONNX", args.onnx_model, ORTModelForCausalLM, args.prompt, args.device, max_new_tokens=100)

if __name__ == "__main__":
    main()

Before submitting

[ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
[x] (N/A) Did you make sure to update the documentation with your changes?

This is a model addition and there is not model-specific documentation that I can find

[x] (N/A) Did you write any new necessary tests?

This is a model addition and there are not model-specific tests that I can find

Who can review?

ONNX / ONNX Runtime : @fxmarty, @echarlaix, @JingyaHuang, @michaelbenayoun

can you add this architecture to the tests suite using a small tiny random model, and push it to the hub (you can create it with GraniteForCausalLM.from_config())

I've created a tiny-random model: https://huggingface.co/hf-internal-testing/tiny-random-GraniteForCausalLM.
Unfortunately, ONNX export currently fails
optimum-cli export onnx -m hf-internal-testing/tiny-random-GraniteForCausalLM /tmp/

with the following error:

See log
Framework not specified. Using pt to export the model.
Automatic task detection to text-generation-with-past (possible synonyms are: causal-lm-with-past).
Using the export variant default. Available variants are:
    - default: The default ONNX variant.

***** Exporting submodel 1/1: GraniteForCausalLM *****
Using framework PyTorch: 2.4.1+cu121
Overriding 1 configuration item(s)
	- use_cache -> True
We detected that you are passing `past_key_values` as a tuple of tuples. This is deprecated and will be removed in v4.47. Please convert your cache or use an appropriate `Cache` class (https://huggingface.co/docs/transformers/kv_cache#legacy-cache-format)
/usr/local/lib/python3.10/dist-packages/transformers/cache_utils.py:447: TracerWarning: Using len to get tensor shape might cause the trace to be incorrect. Recommended usage would be tensor.shape[0]. Passing a tensor of different shape might lead to errors or silently give incorrect results.
  or len(self.key_cache[layer_idx]) == 0  # the layer has no cache
/usr/local/lib/python3.10/dist-packages/transformers/models/granite/modeling_granite.py:982: TracerWarning: Converting a tensor to a Python boolean might cause the trace to be incorrect. We can't record the data flow of Python values, so this value will be treated as a constant in the future. This means that the trace might not generalize to other inputs!
  if sequence_length != 1:
/usr/local/lib/python3.10/dist-packages/transformers/cache_utils.py:432: TracerWarning: Using len to get tensor shape might cause the trace to be incorrect. Recommended usage would be tensor.shape[0]. Passing a tensor of different shape might lead to errors or silently give incorrect results.
  elif len(self.key_cache[layer_idx]) == 0:  # fills previously skipped layers; checking for tensor causes errors
Traceback (most recent call last):
  File "/usr/local/bin/optimum-cli", line 8, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.10/dist-packages/optimum/commands/optimum_cli.py", line 208, in main
    service.run()
  File "/usr/local/lib/python3.10/dist-packages/optimum/commands/export/onnx.py", line 265, in run
    main_export(
  File "/usr/local/lib/python3.10/dist-packages/optimum/exporters/onnx/__main__.py", line 374, in main_export
    onnx_export_from_model(
  File "/usr/local/lib/python3.10/dist-packages/optimum/exporters/onnx/convert.py", line 1188, in onnx_export_from_model
    _, onnx_outputs = export_models(
  File "/usr/local/lib/python3.10/dist-packages/optimum/exporters/onnx/convert.py", line 782, in export_models
    export(
  File "/usr/local/lib/python3.10/dist-packages/optimum/exporters/onnx/convert.py", line 887, in export
    export_output = export_pytorch(
  File "/usr/local/lib/python3.10/dist-packages/optimum/exporters/onnx/convert.py", line 583, in export_pytorch
    onnx_export(
  File "/usr/local/lib/python3.10/dist-packages/torch/onnx/utils.py", line 551, in export
    _export(
  File "/usr/local/lib/python3.10/dist-packages/torch/onnx/utils.py", line 1648, in _export
    graph, params_dict, torch_out = _model_to_graph(
  File "/usr/local/lib/python3.10/dist-packages/torch/onnx/utils.py", line 1174, in _model_to_graph
    graph = _optimize_graph(
  File "/usr/local/lib/python3.10/dist-packages/torch/onnx/utils.py", line 714, in _optimize_graph
    graph = _C._jit_pass_onnx(graph, operator_export_type)
  File "/usr/local/lib/python3.10/dist-packages/torch/onnx/utils.py", line 1997, in _run_symbolic_function
    return symbolic_fn(graph_context, *inputs, **attrs)
  File "/usr/local/lib/python3.10/dist-packages/torch/onnx/symbolic_helper.py", line 292, in wrapper
    return fn(g, *args, **kwargs)
  File "/usr/local/lib/python3.10/dist-packages/torch/onnx/symbolic_opset14.py", line 177, in scaled_dot_product_attention
    query_scaled = g.op("Mul", query, g.op("Sqrt", scale))
  File "/usr/local/lib/python3.10/dist-packages/torch/onnx/_internal/jit_utils.py", line 93, in op
    return _add_op(self, opname, *raw_args, outputs=outputs, **kwargs)
  File "/usr/local/lib/python3.10/dist-packages/torch/onnx/_internal/jit_utils.py", line 244, in _add_op
    inputs = [_const_if_tensor(graph_context, arg) for arg in args]
  File "/usr/local/lib/python3.10/dist-packages/torch/onnx/_internal/jit_utils.py", line 244, in <listcomp>
    inputs = [_const_if_tensor(graph_context, arg) for arg in args]
  File "/usr/local/lib/python3.10/dist-packages/torch/onnx/_internal/jit_utils.py", line 276, in _const_if_tensor
    return _add_op(graph_context, "onnx::Constant", value_z=arg)
  File "/usr/local/lib/python3.10/dist-packages/torch/onnx/_internal/jit_utils.py", line 252, in _add_op
    node = _create_node(
  File "/usr/local/lib/python3.10/dist-packages/torch/onnx/_internal/jit_utils.py", line 312, in _create_node
    _add_attribute(node, key, value, aten=aten)
  File "/usr/local/lib/python3.10/dist-packages/torch/onnx/_internal/jit_utils.py", line 363, in _add_attribute
    return getattr(node, f"{kind}_")(name, value)
TypeError: z_(): incompatible function arguments. The following argument types are supported:
    1. (self: torch._C.Node, arg0: str, arg1: torch.Tensor) -> torch._C.Node

Invoked with: %427 : Tensor = onnx::Constant(), scope: transformers.models.granite.modeling_granite.GraniteForCausalLM::/transformers.models.granite.modeling_granite.GraniteModel::model/transformers.models.granite.modeling_granite.GraniteDecoderLayer::layers.0/transformers.models.granite.modeling_granite.GraniteSdpaAttention::self_attn
, 'value', 1.0
(Occurred when translating scaled_dot_product_attention).

Investigating why 🤔

Upgrading to torch nightly fixes it 👍

Oooh great ! it's the error I've been seeing when exporting clip with sdpa

Thanks for all the quick review! I realized I missed a lot of context on the issue itself, so will update with details of how I tested this locally.

just to confirm, I tested the ONNX model with Transformers.js, and it matches the python version exactly 👍
This PR should be good to merge once we add the tiny random test (https://huggingface.co/hf-internal-testing/tiny-random-GraniteForCausalLM). Not exactly sure how to handle the minimum pytorch version though (cc @IlyasMoutawwakil)

and just to confirm, I tested the ONNX model with Transformers.js, and it matches the python version exactly 👍

Perfect, thanks a lot @xenova

This PR should be good to merge once we add the tiny random test (https://huggingface.co/hf-internal-testing/tiny-random-GraniteForCausalLM). Not exactly sure how to handle the minimum pytorch version though (cc @IlyasMoutawwakil)

For the minimum pytorch version, it can be set with MIN_TORCH_VERSION https://github.com/huggingface/optimum/blob/9fd9ca5505e83f67c2a5e4c4f6f56d5fcf28442f/optimum/exporters/onnx/model_configs.py#L740

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@gabe-l-hart Let me know if you need any help with the final steps (as outlined above)! 🤗 PyTorch 2.5 just released, which should fix the above issue (untested; previous nightly version did fix the bug).

Thank you! I've been sidetracked on other threads, but hope to get back to this one later in the week.

No worries! I'm doing the conversions of the latest set of released models with this branch. Will report back when complete.
Also, looks like https://huggingface.co/ibm-granite/granite-3.0-1b-a400m-instruct is tagged as granitemoe, which we can also do in this PR.

FYI: I can confirm that https://huggingface.co/onnx-community/granite-3.0-2b-instruct exported correctly and works in Transformers.js 👍
The MOE model is slightly more complicated, and throws an error when performing top_k on a tensor whose dim is < k. Looks like we need to insert a min op there.

Ok, I've added MIN_TORCH_VERSION and put "granite" into the mapping in exporters_utils.py.

The MoE model is slightly more complicated, and throws an error when performing top_k on a tensor whose dim is < k. Looks like we need to insert a min op there.

When I looked at this myself, I got stuck on the "more complicated" part and punted on it for this PR. Is adding this min op something that you can point me in the direction on?

you can also add the architecture for testing with onnxruntime 😄

Thanks for pointing that out! I think I've got that added too. I tried running the onnxruntime tests locally but ran myself out of disk space (was already pushing it with so many models!)

Thanks @xenova! Do you have any further insight on what we would need to add to support the MoE models? I haven't been able to dig deep on it and it looked like you had a thought around inserting a min operation somewhere.

Thanks @gabe-l-hart for the PR! Everything looks good in my conversion script too (for Transformers.js models).
As for the MoE models, I think this would require a modification to the transformers modelling code (here I think), just ensuring that the top_k parameter is less than the available number of options. Here is the error message I get when exporting with the same config as granite:
Error: Non-zero status code returned while running TopK node. Name:'/model/layers.0/block_sparse_moe/router/TopK_1' Status Message: k argument [256] should not be greater than specified axis dim value [200]

Thank you! I'll dig in and see if I can get the MoE ones working with tweaks in transformers

Great! The topK operator in ONNX/ONNXRuntime seems to be a bit stricter than the version in pytorch, which seems to be causing the error.

Here's a tiny random model which you can use for testing: https://huggingface.co/hf-internal-testing/tiny-random-GraniteMoeForCausalLM

Awesome, thank you!

it seems that granite tests with onnxruntime are not passing, any idea why ?
=========================== short test summary info ============================
FAILED onnxruntime/test_modeling.py::ORTModelForCausalLMIntegrationTest::test_compare_merged_and_not_merged_models_outputs_09_granite_True - RuntimeError: Error in execution: Non-zero status code returned while running Gather node. Name:'/model/embed_tokens/Gather' Status Message: indices element out of data bounds, idx=35790 must be within the inclusive range [-32000,31999]
FAILED onnxruntime/test_modeling.py::ORTModelForCausalLMIntegrationTest::test_pipeline_ort_model_18_granite_False - onnxruntime.capi.onnxruntime_pybind11_state.InvalidArgument: [ONNXRuntimeError] : 2 : INVALID_ARGUMENT : Non-zero status code returned while running Gather node. Name:'/model/embed_tokens/Gather' Status Message: indices element out of data bounds, idx=35790 must be within the inclusive range [-32000,31999]
FAILED onnxruntime/test_modeling.py::ORTModelForCausalLMIntegrationTest::test_pipeline_ort_model_19_granite_True - RuntimeError: Error in execution: Non-zero status code returned while running Gather node. Name:'/model/embed_tokens/Gather' Status Message: indices element out of data bounds, idx=35790 must be within the inclusive range [-32000,31999]
FAILED onnxruntime/test_modeling.py::ORTModelForCausalLMIntegrationTest::test_compare_with_and_without_past_key_values_09_granite - onnxruntime.capi.onnxruntime_pybind11_state.InvalidArgument: [ONNXRuntimeError] : 2 : INVALID_ARGUMENT : Non-zero status code returned while running Gather node. Name:'/model/embed_tokens/Gather' Status Message: indices element out of data bounds, idx=35790 must be within the inclusive range [-32000,31999]
==== 4 failed, 891 passed, 909 skipped, 3041 warnings in 2021.46s (0:33:41) ====

it seems that granite tests with onnxruntime are not passing on main, any idea why ?

Ah that's an issue with the tiny model. My bad! Will re-export with the correct vocab size (49152, not 32000)

Fixed 👍

Thank you! I've been in-and-out today and hadn't had a chance to look into it. Appreciate the quick fix.

@xenova Can you share the export command you're running that produced this error?
I'm working on the MoE support and I'm not seeing a case where self.top_k is less than the shape on the given dimension. I've tried with both the tiny model and with granite-3.0-1b-a400m-instruct.
The command I'm running is:
optimum-cli export onnx --model /Users/ghart/models/granite-3.0-1b-a400m-instruct /Users/ghart/models/granite-3.0-1b-a400m-instruct-onnx/ --task text-generation-with-past

@xenova Can you share the export command you're running that produced https://github.com/huggingface/optimum/pull/2043#issuecomment-2450852233?

The error occurs during runtime 😅 ... the export process seems to produce a valid .onnx graph, but throws an error when actually running the model.


On the relationship with torchtune

Will it be possible in the future for you to coordinate with the torchtune project so that we are able to use A for xyz and B for ikj?
We've been using litgpt a lot appreciating the nice balance between minimal modeling implementation and key quality of life features for distributed LLM training through the lightning ecosystem.
It seems like since torchtune, lightning, and litgpt are both at varying levels of torch nativeness and the tools for pretraining and finetuning do tend to diverge a bit, it would be cool for us as users of such toolkits if there's an effort to double down on this distinction so that we get optimal utility in all cases without heavy duplication of functionality across libraries.
I already see some nods to litgpt in the torchtune codebase but I wonder how intentional or open the channels of communication are here.
@carmocca
🤝
@rohan-varma

Glad to hear that you've been enjoying LitGPT
And thanks for bringing that up. What we can tell so far is that torchtune was indeed inspired by litgpt in the motivation and the design (e.g., as quick search here shows: https://github.com/search?q=repo%3Apytorch%2Ftorchtune+lit-gpt&type=pullrequests).
Although we haven't yet heard from the torchtune team or about torchtune itself, we are open to collaboration in the spirit of open source. However, the lack of communication makes it challenging to make any definitive statements at this time.
What we will do on our end is continue to work with the community so that others can join the journey or decide to build on or be inspired by our work.

Yep. I performed a similar search of the torchtune repo and that prompted this comment.
Sounds good. I saw a response here a little bit ago, but I will wait for a revised one if there is some news on this.


[BUG] Text output overflows prematurely when window width decreases

Opening a Terminal Large enough to fit the prompt works fine.
However, when the width of the terminal window is reduced enough...
The text overflows to the next line even though it appears as it should fit.
Steps to reproduce the behavior:

Open a Terminal
Reduce width by about half, or as necessary
Watch the text overflow

Expected behavior:
Terminal Content should have not changed
Screenshots

macchina --doctor output
Correct
System Information

Distribution: Manjaro
Desktop Environment: Plasma (with i3 WM)
Operating System: Linux
Terminal: Kitty
Macchina's version: 0.9.2

I'll check this out, thanks for reporting :)

Hey there!
It is possible that your terminal emulator could be the reason this is happening, on Alacritty with my setup (qtile) I don't have such an issue.
Could you try out a different terminal emulator to maybe confirm this for me?

I tried it with Konsole from KDE and the problem is there, but in a different form.
Testing the slower neofetch prompt, it does not have this problem.
In this video, i remove everything from my .zshrc just to be sure it is only Macchina that outputs anything
macchina --custom-ascii ~/.config/macchina/assembly.ascii
https://user-images.githubusercontent.com/5043481/125150945-60793700-e0f8-11eb-8df2-27c8c8599182.mp4
Will attach my ascii art, and my config to see if it might be specific to certain settings
macchina.txt
assembly.ascii.txt
hypercomp.json.txt

Hello!
I'm finding hard to reproduce this annoying little bug on my machine, but I do find that reducing the size can sometimes clip the output:

Still, I'll continue to try and locate the root of the issue. Thanks for your help :+1:

After upgrading our tui dependency to the latest version, v0.15.0, it seems the clipping problem has gone away, so the next release should definitely not have this problem. This could possibly be a bug that was addressed in crossterm which is the terminal backend we're using.
Let's keep this issue open until we can confirm it's sorted.

Ok, let me know when there is something I can compile to see if it addresses this issue.

Oh sure, the tui-issues branch already has the changes in place, so you might want to clone the repo and jump in that specific branch to help me test it out even further.

I'm afraid that running version 0.9.3 did not solve the problem.
I tested with the default provided ascii art, and it has the same issue.
What is interesting however, is in the kitty terminal, highlighting the prompt shows that the terminal thinks there is text to the right of the last visible character. The amount of extra invisible text seems to be the extra length necessary to recreate the original windows size when the terminal spawned.

It seems I can make a terminal window wider the initially spawned and reduce it again no problem, but going any less then what the original width was will cause the text to overflow.

I'm afraid that running version 0.9.3 did not solve the problem.

Well that sucks, it might be that we're doing something wrong before sending the output to be rendered.

Been running XFCE for around 2 weeks now, and I'm finding it hard to replicate this bug still (even on different terminals)... If you're familiar with Rust, consider giving the project a hand :)

I'm afraid I am not familiar with Rust, but will see what I can find.
I would like to know though, the terminals you have tested with, what they do with text when it overflows on the terminal after it shrinks in size?
I think Kitty is configured to wrap to the next line, while it seems other terminals will just truncate the line, I got Alacritty to test this and it just truncates the line, . If this is a deeper problem with terminal incompatibility, I will consider bringing this up to the author of kitty. We have to rule out first that macchina is not placing invisible characters to the end of the line as I suspect in this screenshot https://user-images.githubusercontent.com/5043481/125209970-95e56800-e251-11eb-8dbb-173ef9008b07.png, and is instead using a newline character immediately.

Well my testing thusfar has included alacritty, xfce-terminal (and even kitty) which all (have this problem but) work as intended when the terminal is brought back to a big enough size.
You could always just clear; macchina until this is resolved.

Well my testing thusfar has included alacritty, xfce-terminal (and even kitty) which all (have this problem but) work as intended when the terminal is brought back to a big enough size.

Oh, just to be clear, when I return the terminal to its original size when the window re-spawns, the macchina prompt is fixed again like you say happens on your computer.
The issue is that the prompt should not be affected by resizing the window when there is still plenty of space to show the whole prompt.
For example, my prompt on my monitor takes less then half my screen.

I open a terminal on my Tiling WM and the terminal starts with an initial width of the whole screen.
Then I resize the width smaller by even just one char...
The prompt immediately overflows to the next line, when there is still plenty of space.

Is this the problem you were referring to in your previous comment? Because that is the bug itself.

The issue is that the prompt should not be affected by resizing the window when there is still plenty of space to show the whole prompt.

I agree, this can get pretty annoying.

Then I resize the width smaller by even just one char...

The prompt immediately overflows to the next line, when there is still plenty of space.

This is only an issue when box and ascii are activated right?

It is an issue always, regardless of box or ascii enabled.
Does this mean you were finally able to replicate the bug?

Does this mean you were finally able to replicate the bug?

Yeah I was, it's just not as severe as you have it.
I've taken a bit of a break recently, but I've just started to get back into working on all my projects. I'll start hunting for this bug again.


npx mrm lint-staged does not work after husky v5+

Description

Steps to reproduce

In your npm project, run npx mrm lint-staged
Perform a git commit and you will notice that the hook did not run

Debug Logs
Debug logs irrelevant, this is an issue with the script npx mrm lint-staged.

expand to view

Environment

OS: Ubuntu 20.04.1
Node.js: 12.18.3
lint-staged: 10.5.4

Interim fix:
Had to do a manual downgrade to husky v4:
npm i -D husky@4

Then it worked:

I opened a issue on the mrm repo: https://github.com/sapegin/mrm/issues/145

Are there any plans to make lint-staged compatible with husky v5?

@ali-farooq0 lint-staged itself works fine with husky v5. The mrm setup script is not really part of the project, and just helps set up a configuration.

@iiroj
Gotcha. I couldn't find any documentation to set it up with husky v5. With Husky v5 they've moved execution from the package.json to a stand-alone pre-commit file. What's the best way to invoke lint-stage from it? using npx?
https://typicode.github.io/husky/#/?id=add-a-hook

I would go with the example at Automatic (recommended):
npx husky add .husky/pre-commit 'npx --no-install lint-staged'

Personally I have not had success with upgrading to Husky v5 due to numerous issues, so I've stayed at v4...

Will give that a shot. Thanks!
Yeah for now I've decided to do the same :D


[Feature] Support nacos registry

Search before asking

[X] I had searched in the issues and found no similar issues.

Feature Request
Provide service registry discovery capability of Nacos for EventMesh
Are you willing to submit PR?

[X] Yes I am willing to submit a PR!

Support nacos registry implementation:

Implement the RegistryService interface in spi mode
Added registry service address configuration items, for example:
eventMesh.registry.address=<IP_ADDRESS>:8848
Load the specified registry type in the init method
Perform service registration, heartbeat maintenance, etc. in the start mode
Service logout and resource shutdown in shutdown method

It is better to control the lifecycle of RegistryService in EventMeshServer.

It is better to control the lifecycle of RegistryService in EventMeshServer.

Yes, that's it for now

We also need to consider whether the DEFINITION of the API layer meets the existing requirements, the reporting information interacted with the registry must ensure a certain scalability.

We also need to consider whether the DEFINITION of the API layer meets the existing requirements, the reporting information interacted with the registry must ensure a certain scalability.

nacos should be satisfied at present

Since this the eventmesh has supported regist to nacos， we can consider supporting the reporting of data to the registry for some other features such as #827 @li-xiao-shuang

Since the eventmesh has supported regist to nacos， we can consider supporting the reporting of data to the registry for some other features such as #827 @li-xiao-shuang

What data do I need to register with NACOS?

webhook feature is undering detailed design, after the design we may know what data structrue will be registed to NACOS, you can keep track of this #827


Migrating the database

Updating the database in a live system
First: always make the change in the sequelize JavaScirpt model definition so it will be available in the code as JavaScript objects (ex: registrations/server/db/models/wallet.js).  This makes it much easier to work with the database.
Currently I make the change locally (in my database) then document the DDL (sql changes) in fio-registrations/UPGRADE.md.  This works but complicates upgrades because it is manual.
In any event, include any changes in server/db/models/* in UPGRADE.md..

The sequelize migrations and seed solution may not play well well new installs and with upgrades.  They will tend to run on new installs.  Additionally I had an error in the migration when I tried to run raw DDL.  I think a custom migration solution may be needed if we are to remove the need to update and run DDL from UPGRADE.md.

A migration should either the [major].x.x or x.[minor].x version in package.json (probably the major).


feat: project information history details

Implements #1669

Mostly working great though there seems to be a bug if I upload files on a fresh form where it is showing [object object]. I then added another file and everything shows up perfect. Tried this a couple times so it seems easily reproduceable.

This is fixed now


bugfix/notion fix pagination in BlocksChildrenEndpoint and DatabasesEndpoint

I was wondering why the notion runner was running in circles when ingesting bigger pages (with more than 100 blocks) - turns out the recursive child block getters forgot to point at the next page / cursor

I had asked this get folded into an existing PR that's migrating the notion connector to v2: https://github.com/Unstructured-IO/unstructured-ingest/pull/162

@hardchor Thanks so much! We are just about to merge in a new Notion connector (we're moving all connectors to V2) and will make sure this is in there.


[web] Skia engine fails when using gradient

Repeatedly seeing the following errors
Assertion failed: org-dartlang-sdk:///flutter_web_sdk/lib/_engine/engine/surface/scene_builder.dart:267:12
retainedSurface.isActive || retainedSurface.isReleased
is not true

flutter doctor:
`[√] Flutter (Channel beta, v1.14.6, on Microsoft Windows [Version 10.0.15063], locale
en-US)
[!] Android toolchain - develop for Android devices (Android SDK version 28.0.3)
! Some Android licenses not accepted.  To resolve this, run: flutter doctor
--android-licenses
[√] Chrome - develop for the web
[√] Android Studio (version 3.5)
[!] IntelliJ IDEA Ultimate Edition (version 2017.1)
X Flutter plugin not installed; this adds Flutter specific functionality.
X Dart plugin not installed; this adds Dart specific functionality.
[√] VS Code (version 1.41.1)
[√] Connected device (3 available)`

Hi @iampopal
Could you provide a code sample to reproduce?
Similar to https://github.com/flutter/flutter/issues/50010

Hi @iampopal
Could you provide a minimal code sample when this assertion is failed?
Related to https://github.com/flutter/flutter/issues/50010

This issue occured on my project as well. Downgrading to v1.14.5 fixed the problem. This project is supposed to be delivered soon so I did not get a chance to debug or provide a minimal sample. This code does use gradients, however.
Here is my error log:
The following assertion was thrown during a scheduler callback:
Assertion failed: org-dartlang-sdk:///flutter_web_sdk/lib/_engine/engine/dom_canvas.dart:74:12
paint.shader == null
is not true

When the exception was thrown, this was the stack
throw_ (package:build_web_compilers/src/dev_compiler/s/w/ir/cache/builder/src/out/host_debug/dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart:196:49)
assertFailed (package:build_web_compilers/src/dev_compiler/s/w/ir/cache/builder/src/out/host_debug/dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart:26:3)
[_drawRect] (package:build_web_compilers/src/dev_compiler/s/w/ir/cache/builder/src/out/host_debug/flutter_web_sdk/lib/_engine/engine/dom_canvas.dart:74:28)
drawRRect (package:build_web_compilers/src/dev_compiler/s/w/ir/cache/builder/src/out/host_debug/flutter_web_sdk/lib/_engine/engine/dom_canvas.dart:136:28)
apply (package:build_web_compilers/src/dev_compiler/s/w/ir/cache/builder/src/out/host_debug/flutter_web_sdk/lib/_engine/engine/surface/recording_canvas.dart:865:11)
...
--------------------------------------------------------------------------------

-------- Exception caught by scheduler library ---------------------------------
Assertion failed: org-dartlang-sdk:///flutter_web_sdk/lib/_engine/engine/surface/scene_builder.dart:267:12
retainedSurface.isActive || retainedSurface.isReleased
is not true
--------------------------------------------------------------------------------

... last error repeated

I got the same error: https://github.com/flutter/flutter/issues/50417

This is a duplicate of https://github.com/flutter/flutter/issues/50417.


How can i encrypt a unencrypted realm?

Before one version of my app, i used realm but not encrypt it.
But now ,i want to encrypt it .how can i write migration? or can u give me something code or demo

You can use -[RLMRealm writeCopyToURL:encryptionKey:error:]/Realm.writeCopyToURL(_:encryptionKey:) to write a copy of your Realm with whatever encryption key (or none) you specify.
There's no way to change/add/remove the encryption of a file in-place because other parts of your app could still be accessing that file.


fix typo in release script

fix typo in release script

[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: rkpagadala
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

release/OWNERS [rkpagadala]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rkpagadala: The following test failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-integ-k8s-tests.sh
3e127cfa1a02773be5bccc5d2d9867881a6164a6
link
/test istio-integ-k8s-tests

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@rkpagadala: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-integ-k8s-tests.sh
3e127cfa1a02773be5bccc5d2d9867881a6164a6
link
/test istio-integ-k8s-tests

prow/e2e-mixer-no_auth.sh
3e127cfa1a02773be5bccc5d2d9867881a6164a6
link
/test e2e-mixer-no_auth

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@rkpagadala: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-integ-k8s-tests.sh
3e127cfa1a02773be5bccc5d2d9867881a6164a6
link
/test istio-integ-k8s-tests

prow/e2e-mixer-no_auth.sh
3e127cfa1a02773be5bccc5d2d9867881a6164a6
link
/test e2e-mixer-no_auth

prow/istio-pilot-multicluster-e2e.sh
3e127cfa1a02773be5bccc5d2d9867881a6164a6
link
/test istio-pilot-multicluster-e2e

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.


windowrulev2 not matching title with regex

Already reported ? *

[X] I have searched the existing open and closed issues.

Regression?
No
System Info and Version
 System/Version info
> hyprctl version
Hyprland, built from branch main at commit 918d8340afd652b011b937d29d5eea0be08467f5  (flake.lock: update).
Date: 2024-06-25
Tag: v0.41.2, commits: 918d8340afd652b011b937d29d5eea0be08467f5
flags: (if any)

Description
The following is the results of the hyprctl clients I am trying to match:
Window 1be6d1b0 -> Extension: (Bitwarden - Free Password Manager) - Bitwarden — Mozilla Firefox:
        mapped: 1
        hidden: 0
        at: 964,34
        size: 952,1242
        workspace: 1 (1)
        floating: 0
        pseudo: 0
        monitor: 0
        class: firefox
        title: Extension: (Bitwarden - Free Password Manager) - Bitwarden — Mozilla Firefox
        initialClass: firefox
        initialTitle: Mozilla Firefox
        pid: 4483
        xwayland: 0

and this is the rules I've tried, none f which seem to work:
windowrulev2 = float,title:(Extension.*)

windowrulev2 = float,title:(.*Extension.*)

windowrulev2 = float,title:^Extension.*

windowrulev2 = float,class:(firefox),title:(Extension.*)

How to reproduce
See description
Crash reports, logs, images, videos
No response

float is static and thus will target the initial properties


When I wanted to migrate that program to sequence signal data, I found that @register_model("mbt2018") The compress and decompress methods of class JointAutoregressiveHierarchicalPriors are very complex

When I wanted to migrate that program to sequence signal data, I found that
The compress and decompress methods of class
@register_model("mbt2018")
JointAutoregressiveHierarchicalPriors

are very complex. Why is it so complex compared to the compress function and the decompress function of the previous models?
s = 4  # scaling factor between z and y
kernel_size = 5  # context prediction kernel size
padding = (kernel_size - 1) // 2

y_height = z_hat.size(2) * s
y_width = z_hat.size(3) * s

What is the purpose of the above s? Is it necessary?

y_q = self.gaussian_conditional.quantize(y_crop, "symbols", means_hat)

Why is symbol quantization used here instead of dequantize, which is the opposite of training?

Also, do I have to train the aux loss very small to get similar results for the forward function and the compress and decompress functions?
Thank you very much for your answer!

The autoregression portion requires a loop of many steps at runtime. This is because the needed information for decoding only becomes available as the tensor is being decoded pixel-by-pixel from top-left to bottom-right. In contrast, during training, all the information about the tensor is immediately available, so that "decoding" can be done in one small step.

Autoregressive loop

Also, previous models contain some amount of code for runtime decoding too, but it's hidden inside the EntropyBottleneck and GaussianConditional classes.

z is smaller than y by a factor of s = 2 * 2 since there are 2x conv downsample layers of stride 2. Since the decoder has access to z_hat, it uses the shape of z_hat to determine how big y_hat will be. This is necessary because the y_hat bitstream doesn't directly record the shape of y_hat.
Symbol quantization is more accurate than e.g. additive uniform noise. Additive noise is used during training is to ensure the model is resilient to some amount of quantization error.
For typical image compression models, aux loss usually doesn't need to be that small. See: https://github.com/InterDigitalInc/CompressAI/issues/167#issuecomment-1292682384


(WIP) Handling return type when creating features from DatetimeTimeIndex

This PR fixes the issue reported in #265.
That issue surfaced as a calculation error, but the the underlying problem was incorrectly stacking features that shouldn’t exist. This PR updates the return type so the invalid feature doesn’t get created.

Codecov Report

Merging #266 into master will decrease coverage by 0.01%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master     #266      +/-   ##
==========================================
- Coverage   94.45%   94.44%   -0.02%
==========================================
  Files          71       71
  Lines        7670     7672       +2
==========================================
+ Hits         7245     7246       +1
- Misses        425      426       +1

Impacted Files
Coverage Δ

featuretools/primitives/primitive_base.py
90.51% <66.66%> (-0.33%)
:arrow_down:

Continue to review full report at Codecov.

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

Looks good!


Add tab size specification

Properly, this time.

For the most part, this should be fine to merge.  The only thing is I would rather we used the term „tab width“ instead of „tab size“.

Great!  If you can resolve the merge conflict then I can merge this into master.

Conflict resolved.


cannot join flo games

Try to reset firewall but still not work
https://www.youtube.com/watch?v=usS4ZVk3Aq8

@chennqqi Does this happen occasionally, or are you always unable to join the game?

@fluxxu I can join when w3c lancher version 1.0. Unable to join the game after version 1.0

@fluxxu Have the same problem..do you fix it?help me pls fix it

@chennqqi do you fix it?help me do the same

Still cannot join flo games

We have released the rewritten launcher to help address this issue. You can give it a try.


Update doc

@AryanSharmaa it's good to see that you're interested in contributing to hub, however spam pull requests with a couple of grammatical changes in the readme aren't the way to go. Closing this request. Would like to see more meaningful contributions from you in the future.


Type definition for MaterializeAction

I just wonder why MaterializeAction is not defined in the type definition file. Is there a good reason for this?

Isn't it? I can see it when importing this project from another project:

sorry, my bad. was using 6.3.0.


Simplemempool does not ensure send undecided batches twice

The simplemempool module removes a transaction from the mempool as soon as it has been provided once to the availability module. This means that if the batch is not decided in the end, all the transactions of the mempool will need to be resubmitted by the client.

Solved by #501


Not compilable in Rust 1.1.0

$ cargo run
   Compiling rocksdb v0.0.8
/Users/robin/.cargo/registry/src/github.com-1ecc6299db9ec823/rocksdb-0.0.8/src/merge_operator.rs:168:27: 168:36 error: type mismatch resolving `<&[u8] as core::iter::IntoIterator>::Item == u8`:
 expected &-ptr,
    found u8 [E0271]
/Users/robin/.cargo/registry/src/github.com-1ecc6299db9ec823/rocksdb-0.0.8/src/merge_operator.rs:168         Some(v) => result.extend(v),
                                                                                ^~~~~~~~~
/Users/robin/.cargo/registry/src/github.com-1ecc6299db9ec823/rocksdb-0.0.8/src/merge_operator.rs:168:27: 168:36 help: run `rustc --explain E0271` to see a detailed explanation
/Users/robin/.cargo/registry/src/github.com-1ecc6299db9ec823/rocksdb-0.0.8/src/merge_operator.rs:172:16: 172:26 error: type mismatch resolving `<&[u8] as core::iter::IntoIterator>::Item == u8`:
 expected &-ptr,
    found u8 [E0271]
/Users/robin/.cargo/registry/src/github.com-1ecc6299db9ec823/rocksdb-0.0.8/src/merge_operator.rs:172         result.extend(op);
                                                                                ^~~~~~~~~~
note: in expansion of for loop expansion
/Users/robin/.cargo/registry/src/github.com-1ecc6299db9ec823/rocksdb-0.0.8/src/merge_operator.rs:171:5: 173:6 note: expansion site
/Users/robin/.cargo/registry/src/github.com-1ecc6299db9ec823/rocksdb-0.0.8/src/merge_operator.rs:172:16: 172:26 help: run `rustc --explain E0271` to see a detailed explanation
error: aborting due to 2 previous errors
Could not compile `rocksdb`.

To learn more, run the command again with --verbose.
$ rustc --version
rustc 1.1.0 (35ceea399 2015-06-19)

Hey @anvie! I've been putting off removing the unstable features because I assumed it would be a far more significant change than was actually required, but it was actually not too bad!  I've cut a new release, 0.1.0, which should work with Rust 1.1 stable!  Enjoy!
https://github.com/spacejam/rust-rocksdb/releases/tag/v0.1.0


bad

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
Describe the solution you'd like
A clear and concise description of what you want to happen.
Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
Additional context
Add any other context or screenshots about the feature request here.

There is literally nothing here, I recommend closing this "issue"

There is literally nothing here, I recommend closing this "issue"


Frontmatter support in mdx plugin

I would like to be able to use frontmatter in mdx files.
Describe the solution you'd like
That I can do this:
import AwesomeContent, { frontMatter } from './awesome-content.mdx'

and have frontMatter equal { title: 'Awesome!' } if awesome-content.mdx starts with this:
---
title: Awesome!
---

Describe alternatives you've considered
The only alternative I can see now, is placing a JSON file next to the mdx file with the data I need. I have tried writing a react-static plugin to handle this, but it doesn't seem to be possible - it must be done in the mdx plugin.

Maybe check out https://github.com/DanWebb/jdown and DanWebb's own react-static site as an example?

Thanks for the suggestion, but it seems that then I'm out of the mdx ecosystem, including the remark/rehype plugins?


Fix evaluation queue implementation

Currently, there is a initial, simple implementation of a queue to feed the controller workflow invocation that it should evaluate. However with higher loads of workflow invocations the queue tends to overflow. This is due to two reasons:

Invocations can occur multiple times in the queue
There is no notion of priority
These issues cause the controller not to keep up with the growing queue. Besides, the invocations that need to be evaluated get buried in between the unnecessary invocations.

To resolve this we need an implementation that supports both priorities and ensure that there are no duplicates in the queue. A promising solution to this is to use a priority queue.

#167 should also close this issue, right?

Yes thanks @thenamly 🙂


Add files via upload

Fixes issue
Screenshots
before PR
after PR
Proposed Changes

If your current branch is master, you should choose to create a new branch for your commit and then create a pull request.

 Could you just fork the repo, edit it and then make a pull request?

Hi, I made a new pull request, does it work?


[KBM] Auto-select keymap according to keyboard being typed on

Description of the new feature / enhancement
The feature that is provided by https://github.com/gmcouto/RightKeyboard would be a logical addition to the Keyboard Manager. When starting to type on an external keyboard it makes sense to automatically switch to the appropriate keymap for that keyboard, and back when resuming to type on another keyboard with another matching layout.
Scenario when this would be used?
E.g. when using multilingual keyboards in different incompatible writing systems. I have a Bluetooth Cyrillic keyboard and a normal Latin laptop keyboard. There is a logical one-to-one correspondence between what keyboard is being typed on and what keyboard layout (US-Intl / RUS) to switch to. Doing so manually is annoying and error-prone, even when using the Alt-Shift shortcut provided by Windows.
The https://github.com/gmcouto/RightKeyboard is already a working solution, but conceptually this feature would really fit well in the Keyboard Manager of Powertoys.
Supporting information
No response

Maybe we can add a group management and a shortcut for quick switching ？

Maybe we can add a group management and a shortcut for quick switching ？

+1

+1
Why this isn't already a feature of Windows baffles the mind.


DuckTabs-BasicApp-LeoVersion

In the sense of creating a basic web app I created DuckTabs
It is a funny web app about ducks and rubber ducks! The site consists of three main tabs: DucksOnDeck, DuckShowroom and DuckStore.
DucksOnDeck
Basically shows upcoming incredible ducks!

DuckShowRoom
Consists of a carousel of funny duck pics

DuckStore
It is as the name suggests a store full of ducks, in this case, rubber ducks. You can view the ducks available and add them to your cart!

This added cart modal appears every-time you add an item to your cart. It doesn't allow you to click the background until closed. You can either close it or it will close it self after 10 seconds of being idle.

The site has support in three languages: Portuguese, English and Korean!

User Cart
In the cart section you can view the ducks you have added, their price and quantity. At the bottom the total price is shown!
Your cart is saved for the total of 24 hours, after that it is erased forever

And just to finish, there is an ancient secret hidden in the depths of DuckTabs so explore and hear for its voice. Hear it once, twice or maybe ten times...for you to hear its real voice...

Pó ano há mais :)


Add ... next to the 'Break Lease' on the blob context menu of blob

Storage Explorer Version: 1.8.1_master_20190619.4
Platform/OS: Linux Ubuntu/macOS High Sierra/Windows 10
Architecture: ia32/x64
Commit: 952cf5dc
Regression From: Not a regression
Steps to reproduce:

Expand one normal account ->'Blob Containers' node.
Open one blob container which container one blob at least.
Right click one blob then check the context menu.

Expect Experience:
There shows 'Break Lease...' on the blob context menu.
Actual Experience:
There shows 'Break Lease' on the blob context menu.

Fix in PR

This issue has been fixed on the latest master build 20190724.6. So closed it.


[WIP RAINCATCH-1409] Move types to dependencies and check dependency usage

Motivation
Add @types to dependencies to allow usage from other typescript projects and check whether we're actually using some of the dependencies.
Description
Add depcheck command
Check dependencies on the packages
Move types dependencies to regular deps from devDeps

Changes Unknown when pulling 6ef0fd7b017e98da583c915ac3e040eb228ab7c9 on types-to-deps into ** on master**.

Changes Unknown when pulling 6ef0fd7b017e98da583c915ac3e040eb228ab7c9 on types-to-deps into ** on master**.

I understand the rationale for this change. Really cool to have dependency checker.
tried that with complete clean and it works fine.

@wtrocki In relation to verifying the behavior I think it might be nice to do a new patch release, but if that's so I'd like to defer merging this PR until I do the same dependencies revision on the angularjs repo.
Though there's no @types issue there, we might be able to reduce bundle sizes very nicely by reviewing our dependencies.


Feature Request: Designing a website for "Scrape Up"

Describe the feature
Hi @nikhil25803, I was having an idea for designing a website for promoting "Scrape Up" tool.
Here, we can add all the details, features, showing the steps how to proceed with a demo video. Also, there can be a "Contact page" and "FAQs page", if someone faces a problem, then he/she can first check out the FAQs, if the problem is not resolved, he/she can reach you out easily.
Please assign this issue to me under GSSOC'23, as I want to work on it🙏🙏
Add ScreenShots

Record

[X] I agree to follow this project's Code of Conduct
[X] I'm a GSSOC 23 contributor
[X] I want to work on this issue

We are not looking for a website right now. So closing this PR for now.


Another way to remove creation of temporary files for alembic

Use temporary files and access using fd. Avoid going into alembic's internal APIs.
Closes #70

I still think we should go ahead and use #70. These internal APIs are intended to be used (quote: However, real-world applications will usually end up using more of the internal API, in particular being able to run commands programmatically, as discussed in the section Commands.).
The current fix still crashes (because the filesystem doesn't like tempfile.TemporaryFile()):
Traceback (most recent call last):
  File "/ngshare/ngshare.py", line 863, in <module>
    main()
  File "/ngshare/ngshare.py", line 848, in main
    dbutil.upgrade(args.database)
  File "/ngshare/dbutil.py", line 78, in upgrade
    with _temp_alembic_ini(db_url) as (alembic_ini, fd):
  File "/usr/lib/python3.6/contextlib.py", line 81, in __enter__
    return next(self.gen)
  File "/ngshare/dbutil.py", line 60, in _temp_alembic_ini
    tf = tempfile.TemporaryFile()
  File "/usr/lib/python3.6/tempfile.py", line 729, in TemporaryFile
    prefix, suffix, dir, output_type = _sanitize_params(prefix, suffix, dir)
  File "/usr/lib/python3.6/tempfile.py", line 269, in _sanitize_params
    dir = gettempdir()
  File "/usr/lib/python3.6/tempfile.py", line 437, in gettempdir
    tempdir = _get_default_tempdir()
  File "/usr/lib/python3.6/tempfile.py", line 372, in _get_default_tempdir
    dirlist)
FileNotFoundError: [Errno 2] No usable temporary directory found in ['/tmp', '/var/tmp', '/usr/tmp', '/srv/jupyterhub']

I changed the filesystem to be not read-only, and this has fixed the issue, but other problems like the  import at https://github.com/lxylxy123456/ngshare/blob/6ef2fd3b0ee8bd64d28e70e5508152cd21201b0b/ngshare/alembic/env.py#L18 is still causing issues. Even after I fix all of that, the alembic call takes around 0.25s instead of 0.013s for the alembic_ini_no_temp branch when there is no update. I know this isn't much since it's not called frequently, but I just don't see the point of dealing with temporary files and context managers when we can just use the alembic API.

Use of fd is not desired.


fix: rename sparkexecutor test file

The spark executor test had a misspelling in the file name.

@mrmasterplan - this test actually fails at: File "/dbfs/test/.../tests/cluster/delta/test_sparkexecutor.py", line 42, in test_can_execute.
I can see that you have made this test (that fails):
self.assertEqual(Spark.get().sql('SHOW DATABASES LIKE "my_db2*"').count(), 0)
AssertionError: 1 != 0


Installation steps are needed before setup script

You need to complete "Installation" steps in advance if you want to use the example script. It should be described in the Prerequisites section of README.

Closed since pr is merged.


Add more specific dependencies for the nix build.

gperftools (for tcmalloc) and zlib are also requested
by cmake.
diff and time are used in the regression tests.
Signed-off-by: Henner Zeller<EMAIL_ADDRESS>

CC: @sjkelly

Good catch. The test packages are small, but if you ever need larger and optional dependencies in the shell you can add arguments (right now pkg is the only argument. For example:
{pkgs ? .... ,
 tests ? false}:

pkgs.mkShell {
  buildInputs = with pkgs;
    [
       ...
    ]
    ++ (lib.optional tests my_test_dep1)
    ++ (lib.optional tests my_test_dep2);

And call with nix-shell --arg tests true to enable the specific dependencies for the tests flag.


Dependency injection for Actors

Ass the basis of the Actor project is a .net core webapp we should be able to use the asp.net Dependecy Injection framework. However if you try to use it now the ActorService cannot create the Actor as it is not setup to create actors with other constructors.
see:
https://github.com/dapr/dotnet-sdk/blob/master/src/Dapr.Actors/Runtime/ActorService.cs#L43

@Xorpio When you register your actor, you can provide a Func to create Actor instances, it will be sued to create your actor instances, you can inject through this Func.
actorRuntime.RegisterActor<DemoActor>(                 (type) =>                 new ActorService(type, (actorService, actorId) =>                 new DemoActor(actorService, actorId)));

Great! Missed that one. Works like a charm. Thanks!

I like this approach, but how does it work if you want to inject an instance of ILogger into your actor's constructor?  How do you obtain an instance of ILogger within the CreateHostBuilder method?

I like this approach, but how does it work if you want to inject an instance of ILogger into your actor's constructor? How do you obtain an instance of ILogger within the CreateHostBuilder method?

Hi @neildonkin,
I instead followed the steps provided here https://github.com/dapr/dotnet-sdk/issues/255#issuecomment-624382344 in #255 and it worked. Basically you do not need to inject the ILogger from withing the CreateHostBuilder. It can simply turn into:
.UseActors(actorRuntime =>
                {
                })

And then, together with the actor registration, you define a factory. Not the ideal answer to your question, but maybe you end up achieving the same outcome?
The registration then ends up basically like:
. . .
services.AddActor(MyActorFactory)
. . .
private static MyActor MyActorFactory(ActorService service, ActorId id, IServiceProvider provider) =>
          new MyActor (
              service,
              id,
              provider.GetRequiredService<IMyCustomService>());

Just remember registering your custom services before adding them within the actor factory.

I'm reopening this issue - I was playing with this over the weekend and I didn't end up finding a solution that made me happy. We shouldn't require as much code as https://github.com/dapr/dotnet-sdk/issues/255#issuecomment-624382344 to solve this.
Basic task like getting a logger instance inside an actor are too hard.

Thanks, @rynowak
That's a very nice approach to the problem

@guihlk thank you very much for the additional information and sample code.

Basic task like getting a logger instance inside an actor are too hard.
There's a long history in ASP.NET Core of users having problems with "split brain" when there are two DI containers, two instances of the logging system, etc. Since we provide ASP.NET Core integration this should do the right thing by default.

@guihlk I agree 100% with you on this.  Thanks very much for re-opening this issue.

Basic task like getting a logger instance inside an actor are too hard.
There's a long history in ASP.NET Core of users having problems with "split brain" when there are two DI containers, two instances of the logging system, etc. Since we provide ASP.NET Core integration this should do the right thing by default.

@guihlk I agree 100% with you on this. Thanks very much for re-opening this issue.

thanks mate @neildonkin , but it was @rynowak who reopened the issue hehe

Of course you don't need to have 2 DI containers when using ASP.NET Core ;-)
I agree that it should be available from NuGet package without need to code it again in an app. Maybe I can check the current code and prepare a pull request. I am a little busy, so it may take a week or two...
Are you are interested in?

Summary
Make it easy to use Actors with DI by enabling dependency injection when using ASP.NET Core. In theory the current Actor runtime provides a way to wire up a dependency injection container - but in practice this is too much manual work and it's actually very hard to get the right behavior.
Desired state
Each Actor type has an ActorRegistration which configuration options specific to that Actor type

The actor runtime has an "activator" service that's responsible for creation/destruction of actors
When using ASP.NET Core the actor runtime will have a default "activator" service that uses DI

Each Actor type has a constructor that accepts:

an instance of ActorService that must be passed to the base constructor
arbitrary other parameters to be satisfied from DI
the ActorService acts as a parameter object allows us to bundle all of the functionality that needs to be passed to the base class without further API changes
the ActorId is now part of ActorService
features that are not intended for consumption in an Actor instance are moved elsewhere (from ActorService)

Each Actor Instance gets its own DI scope

Scoped services resolved from DI share the same lifetime as the Actor instance
Disposable transients accessed from the Actor instance will be disposed on Actor deactivation

Example
Here's a design proposal for what the UX would look like:
public class MyCoolActor : Actor, IIceCreamVendor
{
    private readonly IceCreamRepository repo;

    public MyCoolActor(ActorService service, IceCreamRepository repo)
        base(service)
    {
         this.repo = repo;
    }

    public async Task<IceCream> ServeAsync(IceCreamOrder order)
    {
        // do something amazing here with 'this.repo'
    }
}

API Changes
I'll make a list of API changes as I'm going through this and building it:

Actor constructor signature changes (see above)
Remove IActorService (why does it exist)
Changes to properties on ActorService
Changes to how Actors are registered in non-mainline cases

If we can make the IServiceProvider instance get into the ActorSystem, then we can replace this line, which is using Activator.CreateInstance, by ActivatorUtilities.CreateInstance here, which expects you to pass a IServiceProvider as well as some non-injected parameters.
In this line, we could pass the IServiceProvider s to the ActorRuntime, and then the ActorRuntime can pass the provider to the ActorService in this line.
@rynowak If you are on not working on this already, I can have a try this weekend to make a PR for this.


Optional Cache?

I have 27k bookmarks!
'B-C-M' I assume reads them all at startup (Ff hogs cpu for a couple of minutes).
Would it be possible to provide a cache option to avoid re-scans (with a Refresh/Update button for new BMklets) ?
Or ... a way of accessing just BMklets ?

Actually there is not way to get only bookmarklets with the WebExtension API (you need to parse the whole bookmarks tree).
But it's possible to implement such feature (manual refresh only) or use WebExtension bookmarks API with more granularity


Drop requests-wasm-polyfill?

Looking at the latest release of urllib3: https://github.com/urllib3/urllib3/releases/tag/2.2.0
It looks like we might be able to drop requests-wasm-polyfill, since they now seem to be supporting emscripten (not just Pyodide) as an environment, and requests depends on urllib3.
cc @DerThorsten who may also have some insights on this.

Trying this locally quickly, maybe this would first require to package brotli?

Using this test environment.yml:
name: xeus-python-kernel
channels:
  - https://repo.mamba.pm/emscripten-forge
  - conda-forge
dependencies:
  - xeus-python
  - requests
  - urllib3 >=2.2.0

Trying this locally quickly, maybe this would first require to package brotli?

So we seem to have brotli packaged here already: https://github.com/emscripten-forge/recipes/tree/main/recipes/recipes_emscripten/brotli

The name is brotli https://github.com/emscripten-forge/recipes/blob/main/recipes/recipes_emscripten/brotli/recipe.yaml#L5 not brotli-python, I guess that's the reason?

It seems also some versions of urllib3 are pulling brotlipy, it's unclear to me what's the difference with brotli-python, we may want to package that too at some point

So pysocks is defined as an optional in urllib3 here: https://github.com/urllib3/urllib3/blob/23f2287eb526d9384dddeedb6f6345e263bb9b86/pyproject.toml#L49-L51
But not in the feedstock it seems: https://github.com/conda-forge/urllib3-feedstock/blob/0bf0b37a36ce8513397f81457e316a1e1e206001/recipe/meta.yaml#L26
Maybe that's the issue?

I believe we should either get rid of this trick or provide our own recipe. I'll go with with a custom recipe for now and open an issue on the upstream feedstock.

Yeah that sounds like the simplest approach for now :+1:

Looking at the latest release of urllib3: https://github.com/urllib3/urllib3/releases/tag/2.2.0
It looks like we might be able to drop requests-wasm-polyfill, since they now seem to be supporting emscripten (not just Pyodide) as an environment, and requests depends on urllib3.
cc @DerThorsten who may also have some insights on this.

but this still means pyodide? https://github.com/urllib3/urllib3/blob/23f2287eb526d9384dddeedb6f6345e263bb9b86/src/urllib3/contrib/emscripten/fetch.py#L35 or am I missing something?

we could reimplement that part of the pyodide API with pyjs I believe

we could reimplement that part of the pyodide API with pyjs I believe

I have mixed feelings about this.
It could be nice to converge a bit, instead of putting pyodide in core places we could put lower level libraries. Like having those JsArray, JsException,JsProxy,to_js, APIs in a common place for pyodide and emscripten-forge, then use this in urllib3.

we could reimplement that part of the pyodide API with pyjs I believe

I have mixed feelings about this.
It could be nice to converge a bit, instead of putting pyodide in core places we could put lower level libraries. Like having those JsArray, JsException,JsProxy,to_js, APIs in a common place for pyodide and emscripten-forge, then use this in urllib3.

I think this way round this does not work. JsArray, JsException,JsProxy,to_js are implemented with pyodide / pyjs so you cannot just put them somewhere and let pyodide / pyjs consume them.
There should be some absolute minimal API (ie not the full pyodide API, maybe just a subset) which can be implemented by pyodide / pyjs, and some way to use your custom implementation instead of pyodide.
Anyhow, most things used in urlib exist in pyjs in a very similar fashion.

We can switch from an requests-wasm-polyfill to a pyodide-polyfill for now then.

Now getting this when installing requests in an emscripten-forge environment and trying to import it:
---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
Cell In[1], line 1
----> 1 import requests

File /lib/python3.11/site-packages/requests/__init__.py:43
      6 """
      7 Requests HTTP Library
      8 ~~~~~~~~~~~~~~~~~~~~~
   (...)
     38 :license: Apache 2.0, see LICENSE for more details.
     39 """
     41 import warnings
---> 43 import urllib3
     45 from .exceptions import RequestsDependencyWarning
     47 try:

File /lib/python3.11/site-packages/urllib3/__init__.py:209
    193     return _DEFAULT_POOL.request(
    194         method,
    195         url,
   (...)
    204         json=json,
    205     )
    208 if sys.platform == "emscripten":
--> 209     from .contrib.emscripten import inject_into_urllib3  # noqa: 401
    211     inject_into_urllib3()

File /lib/python3.11/site-packages/urllib3/contrib/emscripten/__init__.py:6
      3 import urllib3.connection
      5 from ...connectionpool import HTTPConnectionPool, HTTPSConnectionPool
----> 6 from .connection import EmscriptenHTTPConnection, EmscriptenHTTPSConnection
      9 def inject_into_urllib3() -> None:
     10     # override connection classes to use emscripten specific classes
     11     # n.b. mypy complains about the overriding of classes below
     12     # if it isn't ignored
     13     HTTPConnectionPool.ConnectionCls = EmscriptenHTTPConnection

File /lib/python3.11/site-packages/urllib3/contrib/emscripten/connection.py:17
     15 from ...util.timeout import _DEFAULT_TIMEOUT, _TYPE_TIMEOUT
     16 from ...util.url import Url
---> 17 from .fetch import _RequestError, _TimeoutError, send_request, send_streaming_request
     18 from .request import EmscriptenRequest
     19 from .response import EmscriptenHttpResponseWrapper, EmscriptenResponse

File /lib/python3.11/site-packages/urllib3/contrib/emscripten/fetch.py:34
     31 from importlib.resources import files
     32 from typing import TYPE_CHECKING, Any
---> 34 import js  # type: ignore[import]
     35 from pyodide.ffi import JsArray, JsException, JsProxy, to_js  # type: ignore[import]
     37 if TYPE_CHECKING:

ModuleNotFoundError: No module named 'js'

js is the module of pyodide wrapping the global scope of JavaScript (ie globalThis / window) fyi

Ok. I'll see if I can work something out.

pyjs.js is more or less the same

most of the pyjs api is "documented" here https://github.com/emscripten-forge/pyjs/blob/main/tests/tests/test_pyjs.py

Like having those JsArray, JsException,JsProxy,to_js, APIs in a common place for pyodide and emscripten-forge, then use this in urllib3.

Yeah this was the motivation for opening https://github.com/pyodide/pyodide/issues/3793.

Some progress made:

Expose js and pyodide polyfills https://github.com/emscripten-forge/pyjs/pull/54
Fix empack filtering for urllib3 and certifi https://github.com/emscripten-forge/empack/pull/94 (released ✅)
Missing pysock recipe https://github.com/emscripten-forge/recipes/pull/792 (released ✅)

Once those are complete, we can safely remove the requests-wasm-polyfill and close this

Nice progress, thanks!