Mac all cluster app crashes when tried to pair a second fabric (test app) after first adding in Home app

Problem
Mac all cluster app crashes when tried to pair a second fabric (test app) after first adding in Home app
SHA: 5d8599d195a93dfe5473f64e4f888c322e5df1d2
Steps:

Pair Mac all cluster app in Home app
Open pairing and copy the setup payload
Try pairing with test app Chai and see that Mac all cluster crashes.

Seeing no memory error before the crash.
022-07-12 22:49:02.522291-0700 chip-all-clusters-app[15985:8133943] [all
] 🔴 [1657691342522] [15985:8133943] CHIP: [DMG] Error retrieving data
from clusterId: 0x0000_001D, err = ../../examples/all-clusters-app/linux
/third_party/connectedhomeip/src/system/TLVPacketBufferBackingStore.cpp:
88: CHIP Error 0x0000000B: No memory
[1657691342522] [15985:8133943] CHIP: [DMG] Error retrieving data from
clusterId: 0x0000_001D, err = ../../examples/all-clusters-app/linux/third
_party/connectedhomeip/src/system/TLVPacketBufferBackingStore.cpp:88: CHIP
Error 0x0000000B: No memory
2022-07-12 22:49:02.522301-0700 chip-all-clusters-app[15985:8133943] [all
] 🟢 [1657691342522] [15985:8133943] CHIP: [DMG] RE:Run We cannot put
more chunks into this report. Enable chunking.
[1657691342522] [15985:8133943] CHIP: [DMG] RE:Run We cannot put more
chunks into this report. Enable chunking.
Further down seeing crash with reason noted.
2022-07-12 22:50:39.957514-0700 chip-all-clusters-app[15985:8140025] [all
] 🟢 [1657691439957] [15985:8140025] CHIP: [IN] SecureSession[0x10028a6f0
]: Released - Type:2 LSID:4709
[1657691439957] [15985:8140025] CHIP: [IN] SecureSession[0x10028a6f0]:
Released - Type:2 LSID:4709
Process 15985 stopped

thread #4, queue = 'com.zigbee.chip.framework.controller.workqueue',
stop reason = EXC_BAD_ACCESS (code=2, address=0x100130003)
frame #0: 0x0000000100130003 chip-all-clusters-app`chip::app::WriteHandler:
:ProcessWriteRequest(this=0x0000700003424868, aPayload=0x0000000100c0b730,
aIsTimedWrite=false) at WriteHandler.cpp:621:5
618 	    {
619 	        err = ProcessAttributeDataIBs(AttributeDataIBsReader);
620 	    }
-> 621 	    SuccessOrExit(err);
622 	    SuccessOrExit(err = writeRequestParser.ExitContainer());
623
624 	    if (err == CHIP_NO_ERROR)
Target 0: (chip-all-clusters-app) stopped.
(lldb)

Attached detailed logs for the test.
mac-all-cluster-lldb-logs-crash-after-chai-pairing.txt

@vinay-apple ran this with asan turned on and got:

`==40999==ERROR: AddressSanitizer: heap-use-after-free on address 0x60400012c678 at pc 0x0001003dff2a bp 0x700006c98f90 sp 0x700006c98f88`
WRITE of size 8 at 0x60400012c678 thread T7
    #0 0x1003dff29 in chip::System::TimerList::Remove(chip::System::TimerList::Node*) SystemTimer.cpp:92
    #1 0x1003e4e37 in chip::System::LayerImplSelect::HandleTimerComplete(chip::System::TimerList::Node*) SystemLayerImplSelect.cpp:420
    #2 0x1003e58ef in invocation function for block in chip::System::LayerImplSelect::ScheduleWork(void (*)(chip::System::Layer*, void*), void*) SystemLayerImplSelect.cpp:197
    #3 0x1012b81ba in __wrap_dispatch_async_block_invoke+0xca (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4a1ba)
    #4 0x7fff202b9622 in _dispatch_call_block_and_release+0xb (libdispatch.dylib:x86_64+0x2622)
    #5 0x7fff202ba805 in _dispatch_client_callout+0x7 (libdispatch.dylib:x86_64+0x3805)
    #6 0x7fff202c05e9 in _dispatch_lane_serial_drain+0x25d (libdispatch.dylib:x86_64+0x95e9)
    #7 0x7fff202c10ac in _dispatch_lane_invoke+0x16d (libdispatch.dylib:x86_64+0xa0ac)
    #8 0x7fff202cac0c in _dispatch_workloop_worker_thread+0x32a (libdispatch.dylib:x86_64+0x13c0c)
    #9 0x7fff2046145c in _pthread_wqthread+0x139 (libsystem_pthread.dylib:x86_64+0x345c)
    #10 0x7fff2046042e in start_wqthread+0xe (libsystem_pthread.dylib:x86_64+0x242e)

0x60400012c678 is located 40 bytes inside of 48-byte region [0x60400012c650,0x60400012c680)
freed by thread T7 here:
    #0 0x1012b90c9 in wrap_free+0xa9 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4b0c9)
    #1 0x1003da446 in chip::Platform::MemoryFree(void*) CHIPMem-Malloc.cpp:111
    #2 0x1003f45b4 in void chip::Platform::Delete<chip::System::TimerList::Node>(chip::System::TimerList::Node*) CHIPMem.h:164
    #3 0x1003f4533 in chip::HeapObjectPool<chip::System::TimerList::Node>::ReleaseObject(chip::System::TimerList::Node*) Pool.h:359
    #4 0x1003e50a8 in chip::System::TimerPool<chip::System::TimerList::Node>::Release(chip::System::TimerList::Node*) SystemTimer.h:205
    #5 0x1003e4fa7 in chip::System::LayerImplSelect::CancelTimer(void (*)(chip::System::Layer*, void*), void*) SystemLayerImplSelect.cpp:178
    #6 0x1003e52d3 in chip::System::LayerImplSelect::ScheduleWork(void (*)(chip::System::Layer*, void*), void*) SystemLayerImplSelect.cpp:186
    #7 0x10038ae87 in chip::app::reporting::Engine::ScheduleRun() Engine.cpp:567
    #8 0x10038d03b in chip::app::reporting::Engine::SetDirty(chip::app::AttributePathParams&) Engine.cpp:810
    #9 0x10008af5e in MatterReportingAttributeChangeCallback(unsigned short, unsigned int, unsigned int) ember-compatibility-functions.cpp:1101
    #10 0x1001a3884 in fabricListChanged() operational-credentials-server.cpp:378
    #11 0x1001a410c in emberAfOperationalCredentialsClusterRemoveFabricCallback(chip::app::CommandHandler*, chip::app::ConcreteCommandPath const&, chip::app::Clusters::OperationalCredentials::Commands::RemoveFabric::DecodableType const&) operational-credentials-server.cpp:517
    #12 0x10023094c in chip::app::Clusters::OperationalCredentials::DispatchServerCommand(chip::app::CommandHandler*, chip::app::ConcreteCommandPath const&, chip::TLV::TLVReader&) IMClusterCommandHandler.cpp:1447
    #13 0x1002379d2 in chip::app::DispatchSingleClusterCommand(chip::app::ConcreteCommandPath const&, chip::TLV::TLVReader&, chip::app::CommandHandler*) IMClusterCommandHandler.cpp:2075
    #14 0x1002f8815 in chip::app::InteractionModelEngine::DispatchCommand(chip::app::CommandHandler&, chip::app::ConcreteCommandPath const&, chip::TLV::TLVReader&) InteractionModelEngine.cpp:1025
    #15 0x1002cb7c2 in chip::app::CommandHandler::ProcessCommandDataIB(chip::app::CommandDataIB::Parser&) CommandHandler.cpp:318
    #16 0x1002c772d in chip::app::CommandHandler::ProcessInvokeRequest(chip::System::PacketBufferHandle&&, bool) CommandHandler.cpp:150
    #17 0x1002c611b in chip::app::CommandHandler::OnInvokeCommandRequest(chip::Messaging::ExchangeContext*, chip::PayloadHeader const&, chip::System::PacketBufferHandle&&, bool) CommandHandler.cpp:84
    #18 0x1002f092e in chip::app::InteractionModelEngine::OnInvokeCommandRequest(chip::Messaging::ExchangeContext*, chip::PayloadHeader const&, chip::System::PacketBufferHandle&&, bool, chip::Protocols::InteractionModel::Status&) InteractionModelEngine.cpp:277
    #19 0x1002f461a in chip::app::InteractionModelEngine::OnMessageReceived(chip::Messaging::ExchangeContext*, chip::PayloadHeader const&, chip::System::PacketBufferHandle&&) InteractionModelEngine.cpp:518
    #20 0x1002f4d27 in non-virtual thunk to chip::app::InteractionModelEngine::OnMessageReceived(chip::Messaging::ExchangeContext*, chip::PayloadHeader const&, chip::System::PacketBufferHandle&&) InteractionModelEngine.cpp
    #21 0x100563c4c in chip::Messaging::ExchangeContext::HandleMessage(unsigned int, chip::PayloadHeader const&, chip::BitFlags<chip::Messaging::MessageFlagValues, unsigned int>, chip::System::PacketBufferHandle&&) ExchangeContext.cpp:492
    #22 0x10056c0bb in chip::Messaging::ExchangeManager::OnMessageReceived(chip::PacketHeader const&, chip::PayloadHeader const&, chip::SessionHandle const&, chip::SessionMessageDelegate::DuplicateMessage, chip::System::PacketBufferHandle&&) ExchangeMgr.cpp:320
    #23 0x10058e06d in chip::SessionManager::SecureUnicastMessageDispatch(chip::PacketHeader const&, chip::Transport::PeerAddress const&, chip::System::PacketBufferHandle&&) SessionManager.cpp:590
    #24 0x10058bcb6 in chip::SessionManager::OnMessageReceived(chip::Transport::PeerAddress const&, chip::System::PacketBufferHandle&&) SessionManager.cpp:431
    #25 0x1005a4a57 in chip::TransportMgrBase::HandleMessageReceived(chip::Transport::PeerAddress const&, chip::System::PacketBufferHandle&&) TransportMgrBase.cpp:71
    #26 0x100630eca in chip::Transport::Base::HandleMessageReceived(chip::Transport::PeerAddress const&, chip::System::PacketBufferHandle&&) Base.h:102
    #27 0x1006300cb in chip::Transport::UDP::OnUdpReceive(chip::Inet::UDPEndPoint*, chip::System::PacketBufferHandle&&, chip::Inet::IPPacketInfo const*) UDP.cpp:122
    #28 0x1003c8448 in chip::Inet::UDPEndPointImplSockets::HandlePendingIO(chip::BitFlags<chip::System::SocketEventFlags, unsigned char>) UDPEndPointImplSockets.cpp:688
    #29 0x1003c676b in invocation function for block in chip::Inet::UDPEndPointImplSockets::BindImpl(chip::Inet::IPAddressType, chip::Inet::IPAddress const&, unsigned short, chip::Inet::InterfaceId) UDPEndPointImplSockets.cpp:218

previously allocated by thread T7 here:
    #0 0x1012b8f80 in wrap_malloc+0xa0 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4af80)
    #1 0x1003da310 in chip::Platform::MemoryAlloc(unsigned long) CHIPMem-Malloc.cpp:91
    #2 0x1003eab2e in chip::System::TimerList::Node* chip::Platform::New<chip::System::TimerList::Node, chip::System::Layer&, std::__1::chrono::duration<unsigned long long, std::__1::ratio<1l, 1000l> >&, void (*&)(chip::System::Layer*, void*), void*&>(chip::System::Layer&, std::__1::chrono::duration<unsigned long long, std::__1::ratio<1l, 1000l> >&, void (*&)(chip::System::Layer*, void*), void*&) CHIPMem.h:145
    #3 0x1003ea96a in chip::System::TimerList::Node* chip::HeapObjectPool<chip::System::TimerList::Node>::CreateObject<chip::System::Layer&, std::__1::chrono::duration<unsigned long long, std::__1::ratio<1l, 1000l> >&, void (*&)(chip::System::Layer*, void*), void*&>(chip::System::Layer&, std::__1::chrono::duration<unsigned long long, std::__1::ratio<1l, 1000l> >&, void (*&)(chip::System::Layer*, void*), void*&) Pool.h:324
    #4 0x1003e481c in chip::System::TimerPool<chip::System::TimerList::Node>::Create(chip::System::Layer&, std::__1::chrono::duration<unsigned long long, std::__1::ratio<1l, 1000l> >, void (*)(chip::System::Layer*, void*), void*) SystemTimer.h:194
    #5 0x1003e53ac in chip::System::LayerImplSelect::ScheduleWork(void (*)(chip::System::Layer*, void*), void*) SystemLayerImplSelect.cpp:188
    #6 0x10038ae87 in chip::app::reporting::Engine::ScheduleRun() Engine.cpp:567
    #7 0x10038d03b in chip::app::reporting::Engine::SetDirty(chip::app::AttributePathParams&) Engine.cpp:810
    #8 0x10008af5e in MatterReportingAttributeChangeCallback(unsigned short, unsigned int, unsigned int) ember-compatibility-functions.cpp:1101
    #9 0x1001a3884 in fabricListChanged() operational-credentials-server.cpp:378
    #10 0x1001b7f5d in OpCredsFabricTableDelegate::OnFabricDeletedFromStorage(chip::FabricTable&, unsigned char) operational-credentials-server.cpp:396
    #11 0x1005e74c0 in chip::FabricTable::Delete(unsigned char) FabricTable.cpp:851
    #12 0x1001a48a7 in (anonymous namespace)::DeleteFabricFromTable(unsigned char) operational-credentials-server.cpp:275
    #13 0x1001a3fd7 in emberAfOperationalCredentialsClusterRemoveFabricCallback(chip::app::CommandHandler*, chip::app::ConcreteCommandPath const&, chip::app::Clusters::OperationalCredentials::Commands::RemoveFabric::DecodableType const&) operational-credentials-server.cpp:513
    #14 0x10023094c in chip::app::Clusters::OperationalCredentials::DispatchServerCommand(chip::app::CommandHandler*, chip::app::ConcreteCommandPath const&, chip::TLV::TLVReader&) IMClusterCommandHandler.cpp:1447
    #15 0x1002379d2 in chip::app::DispatchSingleClusterCommand(chip::app::ConcreteCommandPath const&, chip::TLV::TLVReader&, chip::app::CommandHandler*) IMClusterCommandHandler.cpp:2075
    #16 0x1002f8815 in chip::app::InteractionModelEngine::DispatchCommand(chip::app::CommandHandler&, chip::app::ConcreteCommandPath const&, chip::TLV::TLVReader&) InteractionModelEngine.cpp:1025
    #17 0x1002cb7c2 in chip::app::CommandHandler::ProcessCommandDataIB(chip::app::CommandDataIB::Parser&) CommandHandler.cpp:318
    #18 0x1002c772d in chip::app::CommandHandler::ProcessInvokeRequest(chip::System::PacketBufferHandle&&, bool) CommandHandler.cpp:150
    #19 0x1002c611b in chip::app::CommandHandler::OnInvokeCommandRequest(chip::Messaging::ExchangeContext*, chip::PayloadHeader const&, chip::System::PacketBufferHandle&&, bool) CommandHandler.cpp:84
    #20 0x1002f092e in chip::app::InteractionModelEngine::OnInvokeCommandRequest(chip::Messaging::ExchangeContext*, chip::PayloadHeader const&, chip::System::PacketBufferHandle&&, bool, chip::Protocols::InteractionModel::Status&) InteractionModelEngine.cpp:277
    #21 0x1002f461a in chip::app::InteractionModelEngine::OnMessageReceived(chip::Messaging::ExchangeContext*, chip::PayloadHeader const&, chip::System::PacketBufferHandle&&) InteractionModelEngine.cpp:518
    #22 0x1002f4d27 in non-virtual thunk to chip::app::InteractionModelEngine::OnMessageReceived(chip::Messaging::ExchangeContext*, chip::PayloadHeader const&, chip::System::PacketBufferHandle&&) InteractionModelEngine.cpp
    #23 0x100563c4c in chip::Messaging::ExchangeContext::HandleMessage(unsigned int, chip::PayloadHeader const&, chip::BitFlags<chip::Messaging::MessageFlagValues, unsigned int>, chip::System::PacketBufferHandle&&) ExchangeContext.cpp:492
    #24 0x10056c0bb in chip::Messaging::ExchangeManager::OnMessageReceived(chip::PacketHeader const&, chip::PayloadHeader const&, chip::SessionHandle const&, chip::SessionMessageDelegate::DuplicateMessage, chip::System::PacketBufferHandle&&) ExchangeMgr.cpp:320
    #25 0x10058e06d in chip::SessionManager::SecureUnicastMessageDispatch(chip::PacketHeader const&, chip::Transport::PeerAddress const&, chip::System::PacketBufferHandle&&) SessionManager.cpp:590
    #26 0x10058bcb6 in chip::SessionManager::OnMessageReceived(chip::Transport::PeerAddress const&, chip::System::PacketBufferHandle&&) SessionManager.cpp:431
    #27 0x1005a4a57 in chip::TransportMgrBase::HandleMessageReceived(chip::Transport::PeerAddress const&, chip::System::PacketBufferHandle&&) TransportMgrBase.cpp:71
    #28 0x100630eca in chip::Transport::Base::HandleMessageReceived(chip::Transport::PeerAddress const&, chip::System::PacketBufferHandle&&) Base.h:102
    #29 0x1006300cb in chip::Transport::UDP::OnUdpReceive(chip::Inet::UDPEndPoint*, chip::System::PacketBufferHandle&&, chip::Inet::IPPacketInfo const*) UDP.cpp:122

Thread T7 created by T4 here:
    <empty stack>

Thread T4 created by T3 here:
    <empty stack>

Thread T3 created by T0 here:
    <empty stack>

SUMMARY: AddressSanitizer: heap-use-after-free SystemTimer.cpp:92 in chip::System::TimerList::Remove(chip::System::TimerList::Node*)
Shadow bytes around the buggy address:
  0x1c0800025870: fa fa fd fd fd fd fd fd fa fa fd fd fd fd fd fd
  0x1c0800025880: fa fa fd fd fd fd fd fd fa fa fd fd fd fd fd fd
  0x1c0800025890: fa fa fd fd fd fd fd fd fa fa fd fd fd fd fd fd
  0x1c08000258a0: fa fa fd fd fd fd fd fd fa fa fd fd fd fd fd fd
  0x1c08000258b0: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fd
=>0x1c08000258c0: fa fa fd fd fd fd fd fd fa fa fd fd fd fd fd[fd]
  0x1c08000258d0: fa fa 00 00 00 00 00 04 fa fa 00 00 00 00 00 00
  0x1c08000258e0: fa fa 00 00 00 00 00 00 fa fa 00 00 00 00 00 04
  0x1c08000258f0: fa fa 00 00 00 00 00 00 fa fa 00 00 00 00 00 04
  0x1c0800025900: fa fa 00 00 00 00 00 00 fa fa 00 00 00 00 00 04
  0x1c0800025910: fa fa 00 00 00 00 00 00 fa fa 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb

@kpschoedel anything jumping out at you there?

Actually, this is in fact a Darwin-specific problem.  I'm a little surprised we are not hitting in CI all the time; digging into why.
LayerImplSelect::ScheduleWork does the following:

Cancel the existing timer for the completion function and appState.  This is actually pretty questionable, in case the caller in fact meant to schedule the work twice...  This destroys the timer.
Create a timer and add it to the timer list.
On Darwin only, schedule a special "call HandleTimerComplete on this timer" bit, that does not get canceled if the timer is later canceled.

So if we ScheduleWork(func, closure) twice with the same arguments, then on Darwin we end up in a situation where the timer from the first call is destroyed by the second calle, and then eventually we have a use-after-free.
It looks like this code dates back to #7405 @vivien-apple

And yes, if I just comment out the !intersectsInterestPath check in reporting::Engine::SetDirty, I get the ASAN failure above on the above-linked SHA when running the CI "Remove fabric" test.


"No uninstallers were selected" bug

Hello, i am very happy with this software but from time to time, when i try to uninstall a program, i get error that says there were no uninstallers selected.

Kind of annoying bug.

Are you sure you didn't deselect items on the list by accident? If you use checkboxes make sure that they are checked.

Yes, i am sure. i  check item with checkbox. It does not happen all the time, just sometimes.

Here is another screenshot with checked app. Also when i tried to uncheck that app, i could not do it. Other programs i can check and uncheck, but not that i checked previously.

I can see the issue randomly pop up when messing with the groups (e.g. clicking on the group to select all, then selecting some items), but I couldn't find what exactly was wrong. It looks like a regression caused by moving to .NET 5/6.

Same problem for me.


LoRA Fine Tuning

This is just the first draft so we can start building this feature.

Added dataloader.py, which loads data for training
Added train.py, with the current training loop
Added lora.py, for LoRA wrapper of the stage 1 Transformer
Added dummy_dataset folder with 25 data samples to work with when testing (VCTK-->p311)
Commented out the initial inference code when stage 1 model is built.

There is no batch processing in the training loop currently (was getting some dimension mismatching in the KVCache.update function, probably not that difficult to solve).
The dataloader works fine, but everything else requires work. This is just a dirty initial draft so we can start working on this thing together! :-)
Hope to hear some insights! I have time to put into this feature, so any pointers would be great! Am not super well-versed in AI, so bear with me.

I'm going to try getting a very basic thing to run. Currently, there are a couple issues:

LoRA parameter sizes don't match properly with the token embeddings and output layers. This causes tensor dimensions mismatch error. Also, not sure if these are the two layers we actually want to add LoRAs to. Probably need to test a lot, including figuring what the rank value should be.
def _train_infer_stage_one breaks

Can make it clean once we've got a basic loop running successfully

I'm going to try getting a very basic thing to run. Currently, there are a couple issues:
LoRA parameter sizes don't match properly with the token embeddings and output layers. This causes tensor dimensions mismatch error. Also, not sure if these are the two layers we actually want to add LoRAs to. Probably need to test a lot, including figuring what the rank value should be.
def _train_infer_stage_one breaks
Can make it clean once we've got a basic loop running successfully

Got you. Are you not currently having to load all the models onto GPU VRAM though? Given the constraints of a 16GB VRAM, I would probably:

only work with the first stage (this should reduce memory usage significantly as the diffusion model is ~5GB itself)
swap out the Adam optimiser with a SGD optimiser (just to get things running - this should reduce memory usage significantly)
finetune only the first layer, ignore the token embeddings/logit output layer

I'm going to try getting a very basic thing to run. Currently, there are a couple issues:
LoRA parameter sizes don't match properly with the token embeddings and output layers. This causes tensor dimensions mismatch error. Also, not sure if these are the two layers we actually want to add LoRAs to. Probably need to test a lot, including figuring what the rank value should be.
def _train_infer_stage_one breaks
Can make it clean once we've got a basic loop running successfully

Got you. Are you not currently having to load all the models onto GPU VRAM though? Given the constraints of a 16GB VRAM, I would probably:

only work with the first stage (this should reduce memory usage significantly as the diffusion model is ~5GB itself)
swap out the Adam optimiser with a SGD optimiser (just to get things running - this should reduce memory usage significantly)
finetune only the first layer, ignore the token embeddings/logit output layer

Hey @vatsalaggarwal, these are all very helpful insights - much appreciated!
I will implement the points you mentioned:

Only work with first stage
Modify dataloader to return the first two hierarchies of the encodec tokens
Swap over to SGD optimizer to save on memory
Only fine tune the first layer of the first stage to adapt to new speakers (the use case)

Makes sense to just work with stage 1 with cross entropy loss, I just wasn't able to get there - so thanks for that insight.
Should be able to commit these changes later today or tomorrow!

I just wasn't able to get there

Not sure what you mean, but does https://github.com/karpathy/nanoGPT/blob/master/model.py#L184 help?

Makes sense to just work with stage 1 with cross entropy loss, I just wasn't able to get there - so thanks for that insight.

Not sure what you mean, but does https://github.com/karpathy/nanoGPT/blob/master/model.py#L184 help?

It massively helped - I've not worked with transformer networks before (only diffusion & GANs), so studying nanoGPT after you sent it today was very helpful!
Am just making a few more changes for today, then I'm pushing an update :-)

Just committed - here's an overview:

Switched from Adam to SGD optimizer
Modified DataLoader to return first two Encodec token hierarchies as a flattened interleaved tensor (let me know if that looks ok to you?)
Modified LoRA wrapper to only fine tune speaker_cond_pos layer

In nanoGPT-LoRA they fine tune the causal attention layer (https://github.com/danielgrittner/nanoGPT-LoRA/blob/master/model.py#L128). Would it be worth trying something similar with the attention layer here?

Modified training loop

Forward pass entire batch at a time
Wasn't able to format input_pos as (B, T) due to tensor dimension mismatch in KVCache. Unsure if this causes downstream effects or if that's fine?
Loss function call causes error right now. We need to match the GT labels with the model probs. I need some direction here, because I'm not quite sure about what the "unit" of the model's raw output is and what format we need the labels and model outputs to have to correctly calculate the loss. I assumed that the model would output something like a probability distribution of (B, S, V), B=batch_size, S=gen_encodec_tokens_count, V=vocab_size. But it's (B, T, V), with T=text_prompt_token_count. Not sure exactly how to get the data into the right format here for loss calculation, but the remainder of this implementation should be straightforward once this works.

I might start understanding.
Would the idea be to generate a single token at a random location by giving the model:
 +  as the input
E.g. concat (B, P) and (B, T_random) and use to generate (B, V) probs logits for B tokens per batch?
Then input_pos = torch.arange(0, P+T_rand)
with input_pos determined based on the prompt + random number of encodec tokens.
This would then give us B prediction tensors (B, V), for which we will calculate the loss for that batch.
Then the GT labels would be (B, 1), where we take (P+T_rand) on the 2nd dimension (eg the next token in the sequence)?
Is that the right intuition?

I've modified the training loop to be as I described above, and it seems to work.
Somewhere I have made a mistake, causing tensors to be retained in the computational graph across batches, which leads to:
"RuntimeError: Trying to backward through the graph a second time"
Let me know any thoughts! :-)

Still some work to do in dataloader to ensure proper windows (aka blocks in nanoGPT) of X,Y data is prepared for good training.
Haven't been able to solve the error which occurs when calling .backward for the 2nd time:
"RuntimeError: Trying to backward through the graph a second time (or directly access saved tensors after they have already been freed)."
I'm sure it's a very obvious error I've made, but I have not been able to isolate it.

I think I've isolated the backward error (see above) to the caching mechanism in the model. Will work on a solution today and then we should be able to have something training

in the middle of finishing something, haven't had time to look at this, will do it soon, sorry!

in the middle of finishing something, haven't had time to look at this, will do it soon, sorry!

Completely understand, no pressure! I'm just posting updates for whenever you have the time :-)

the updates are super helpful though!
meanwhile I think @lucapericlp and @kaushal07wick are also both keen to make finetuning work, maybe they can help

I have the model training the LoRA layers now, but the data preparation process is currently garbage.
Gonna draw some inspiration from nanoGPT and improve the data preparation, and then we're getting close to running some tests.

@vatsalaggarwal Whenever you have the time, would you be able to help me clear up my current misunderstanding? Solving this last piece would help me finish the PR quickly as the model is training, but I am not properly understanding the meaning behind some of the variables we're working with.
def sample(
    logits,
    guidance_scale: torch.Tensor,
    temperature: torch.Tensor,
    top_p: Optional[torch.Tensor] = None,
    top_k: Optional[torch.Tensor] = None,
):
    # (b, t, vocab_size) <--- is b batch size here? or is it the 2 predicted encodec hierarchies?
    logits = logits[:, -1]
    logits_cond, logits_uncond_spkemb = logits.split(logits.size(0) // 2, dim=0)
    logits = guidance_scale * logits_cond + (1 - guidance_scale) * logits_uncond_spkemb
    probs = logits_to_probs(logits[0], temperature=temperature, top_p=top_p, top_k=top_k)
    idx_next = multinomial_sample_one_no_sync(probs)
    return idx_next, probs

For this input (b, t, vocab_size), would b be the 2 predicted hierarchies of encodec tokens and not the batch size? One hierarchy conditioned on the speaker, and one not conditioned on the speaker? If b represents the number of predicted hierarchies and not the batch size, then I'm completely with you. If that is not the case, could you help me understand this?
Moving on, during inference-time I would like to understand what happens during forwarding of a batch of size B:
def forward(self, idx: Tensor, spk_emb: Tensor, input_pos: Tensor, targets: Tensor = None, debug_mode = False) -> Tensor:
    mask = self.causal_mask[None, None, input_pos]

    x = (
        self.tok_embeddings(idx)
        + self.pos_embeddings(input_pos)
        + self.speaker_cond_pos(spk_emb) * self.spk_cond_mask
    )

    for layer in self.layers:
        x = layer(x, input_pos, mask)
    x = self.norm(x)
    logits = self.output(x)

    if targets is not None:
        # dummy variables for testing
        top_p = 0.95
        guidance_scale = 3.0
        temperature = 0.25
        temperature = torch.tensor(temperature, device=self.device, dtype=self.precision)
        top_k = None
        top_p = torch.tensor(top_p, device=self.device, dtype=self.precision)
        guidance_scale = torch.tensor(guidance_scale, device=self.device, dtype=self.precision)
        # logits is (B, T, V)
        # We need to swap the 2nd and 3rd dimensions to calculate loss
        # (B, T, V) -> (B, V, T)

        # Generate output token idx just for testing
        out_idx, probs = sample(
            logits,
            guidance_scale,
            temperature,
            top_p,
            top_k,
        ) # out_idx is 1 number from 0 to 2561 (vocab size).

        # Swap the 2nd and 3rd dimensions but keep same value positions
        logits_tmp = logits.permute(0, 2, 1)

        if debug_mode:
            print("prompt last idx: ", idx[:, -1]) # (B, 1)
            print("last target idx: ", targets[:, -1]) # (B, 1)
            print("out_idx: ", out_idx) # (1)? Would expect (B, 1) here, like the inputs.

        loss = F.cross_entropy(logits_tmp, targets)
        return logits, loss

    return logits, None

Here, the input arguments I pass to the model forward during training are:
# Prompt, representing tokenized text prompt + random number of flattened interleaved encodec tokens
idx --> (B, S)

# Speaker embedding, isn't causing any trouble
spk_emb --> (B, 1, 256)

# Input position, representing the number of predictions to make
input_pos -->(S,) # Setting this to (B, S) causes the KVCache and Attention layers to fail during inference (dimension mismatch)

# Targets, or GT labels used to calculate loss. This is the same as X, but with the random window of Encodec tokens shifted forward by 1 (eg the next token in the sequence)
targets --> (B, S)

My intuition is wrong here, and the output of the model is (2, S, V) where I would expect (B, 2, S, V). But I am clearly missing something, given that we're supposed to process B tokens in parallel for the batch. Could you help me understand?
My gut feeling is that input_pos is supposed to be (B, S) for the model to predict B tokens. I tried this, but it causes dimension mismatch in KVCache and Attention.
I will work on making KVCache and Attention compatible with a (B, S) input_pos, but you know much more about this than me, so maybe you could share your insight on this?

For this input (b, t, vocab_size), would b be the 2 predicted hierarchies of encodec tokens and not the batch size?

b is batch size, t is timesteps, vocab_size is vocab_size... the hierarchies are flattened and interleaved into one for the first stage model

My intuition is wrong here, and the output of the model is (2, S, V) where I would expect (B, 2, S, V)

does my previous comment help?

I tried this, but it causes dimension mismatch in KVCache and Attention
I will work on making KVCache and Attention compatible with a (B, S) input_pos, but you know much more about this than me, so maybe you could share your insight on this?

KVCaching isn't relevant during training, that should be switched off...

I think I've isolated the backward error (see above) to the caching mechanism in the model.

this shouldn't be used during training

Haven't been able to solve the error which occurs when calling .backward for the 2nd time

Do you mean for the second iteration? Have you zeroed gradients?

I might start to understand a little bit here.
Would the idea be to generate a single token at a random location for each batch by giving the model:
(prompt) + (random number of GT encodec tokens) as the input?
So we would concat (B, P) and (B, T_random) prompt and encodec tokens respectively, and then use that to generate (B, V) tensor logits for B tokens per batch?
Then input_pos = torch.arange(0, P+T_rand) for a given T_rand ∈ [0, T_max - 1].
This would then give us prediction tensor for the batch (B, V), for which we would calculate the loss.
Then the GT labels would be (B, 1), where we take (P+T_rand) on the 2nd dimension from the GT encodec tokens (the next GT token in the sequence that the model is supposed to predict)?
Is that the right intuition?

Didn't get this... best thing might be to work through the NanoGPT training loop... but rough idea is to create a row of (B, S) which contains all the text and audio tokens concatenated together. Then, you apply next-token pred. This does the right thing because of causal masking.

I think I've isolated the backward error (see above) to the caching mechanism in the model.

this shouldn't be used during training

Haven't been able to solve the error which occurs when calling .backward for the 2nd time

Do you mean for the second iteration? Have you zeroed gradients?

I might start to understand a little bit here.
Would the idea be to generate a single token at a random location for each batch by giving the model:
(prompt) + (random number of GT encodec tokens) as the input?
So we would concat (B, P) and (B, T_random) prompt and encodec tokens respectively, and then use that to generate (B, V) tensor logits for B tokens per batch?
Then input_pos = torch.arange(0, P+T_rand) for a given T_rand ∈ [0, T_max - 1].
This would then give us prediction tensor for the batch (B, V), for which we would calculate the loss.
Then the GT labels would be (B, 1), where we take (P+T_rand) on the 2nd dimension from the GT encodec tokens (the next GT token in the sequence that the model is supposed to predict)?
Is that the right intuition?

Didn't get this... best thing might be to work through the NanoGPT training loop... but rough idea is to create a row of (B, S) which contains all the text and audio tokens concatenated together. Then, you apply next-token pred. This does the right thing because of causal masking.

Thanks for explaining, that makes sense. I've implemented it, similar to NanoGPT. Pushing momentarily.

The LoRA layer (1st layer in model) is 98k trainable parameters. Will try training now to validate the current code.

Currently sweeping learning rate and LoRA rank, alpha, dropout. Graphs look like this so far:

I'm unsure whether the data is fed into the model properly @vatsalaggarwal. I measure very high loss on the frozen foundational model (loss 8 - 12) when evaluating the model, but the audio sounds just fine. During training of LoRA layers I can reduce loss down to 4-5.
The data is prepared like in nanoGPT but with audio added after the tokenized text:

Loop through all text & wav files of dataset
Tokenize text, extract encodec tokens from wav, take only first 2 hierarchies of encodec tokens
Sequence = <tokenized text + encodec tokens + EOS> concatted. Here, I have tried EOS=1024 and EOS = 2048, but it didn't make a big difference on loss measurements. I also tried appending EOS between the tokenized text and encodec tokens, but this also made little difference in the loss evaluation on the foundational model.
Concat Sequence to the FULL dataset tensor (eg one huge 1-dimensional tensor)

During Training:

A sliding window of block_size length randomly selects a window from the full dataset tensor, feeds it into the model and performs next-token pred.
Loss is calculated with Cross Entropy on the raw logits, and the target idx obtained from shifting the input window +1 to the right to get the next window that the model is expected to predict, like in nanoGPT.

Is this the same way that the foundational model was trained @vatsalaggarwal? I would just expect the loss to start out very low, given its ability to generalize extremely well. Were there any nuances in the way you trained the model?

I think @lucapericlp is close to turning this into a working solution (without LoRA), so might be better to add LoRA to that... hopefully should be out by EOD.
The way you mentioned works for training text-only model, but we had some trouble training text+audio models that way... so the data formats etc were slightly differently, hopefully upcoming PR should clarify!

I think @lucapericlp is close to turning this into a working solution (without LoRA), so might be better to add LoRA to that... hopefully should be out by EOD.
The way you mentioned works for training text-only model, but we had some trouble training text+audio models that way... so the data formats etc were slightly differently, hopefully upcoming PR should clarify!

That is great to hear! I am very interested to see how the solution looks.
Should quickly be able to follow up with added LoRAs to @lucapericlp's solution once pushed. Can run some sweeps to find us the best configuration as well :-)
Thanks!

@danablend / @makorihi check out https://github.com/metavoiceio/metavoice-src/pull/93#pullrequestreview-1934338050

@danablend I think the main thing to keep in mind is: each row is composed of "text tokens | audio tokens | padding"... I think you were taking arbitrary segments of these (per NanoGPT) as that's what they do in the text-LLM trainings, but it doesn't work so great for text->speech training..

@danablend I think the main thing to keep in mind is: each row is composed of "text tokens | audio tokens | padding"... I think you were taking arbitrary segments of these (per NanoGPT) as that's what they do in the text-LLM trainings, but it doesn't work so great for text->speech training..

looking at the README in the other PR, I see
audio_files|captions
./data/audio.wav|./data/caption.txt

but you mention there's a | padding necessary in the finetune dataset as well?

@vatsalaggarwal

@danablend I think the main thing to keep in mind is: each row is composed of "text tokens | audio tokens | padding"... I think you were taking arbitrary segments of these (per NanoGPT) as that's what they do in the text-LLM trainings, but it doesn't work so great for text->speech training..

looking at the README in the other PR, I see
audio_files|captions
./data/audio.wav|./data/caption.txt

but you mention there's a | padding necessary in the finetune dataset as well? or is that post-ingestion within the system

I believe padding is appended in the data preparation step here. Padding token is 2048 as set here

I have gotten LoRAs to train based off of @lucapericlp's awesome work.
Gonna clean it up and prepare for review. Probably going to happen in the weekend.

I have gotten LoRAs to train based off of @lucapericlp's awesome work.
Gonna clean it up and prepare for review. Probably going to happen in the weekend.

NICE! what sort of losses are you getting, and audio?
Did you use https://github.com/huggingface/peft, or did you wrap your own as in this PR in the end?

I have gotten LoRAs to train based off of @lucapericlp's awesome work.
Gonna clean it up and prepare for review. Probably going to happen in the weekend.

NICE! what sort of losses are you getting, and audio?
Did you use https://github.com/huggingface/peft, or did you wrap your own as in this PR in the end?

I've seen losses down at around 0.42 with 15m parameters, just on the last block's attention layer. I haven't actually written the code that loads the model with added LoRA layers yet, so I'm going to get some audio samples once that's there.
I took the LoRA from earlier, which is an adaptation of the one from nanoGPT-LoRA. Would you prefer if I used the Peft library from HF? Could probably change it without too much hassle. It's no so much extra code, now that the fine tuning & data loading has been cracked.

@lucapericlp is the LoRA "master"... what do you think we should?

Re LoRA suggestions, from my experience, the two most impactful factors when comparing vs a full param finetune were:

adapting all layers
training for longer (increasing batch_size with any newly freed VRAM from decrease in learnable params makes this more feasible)

With those insights, I was able to get within a < 1.0pp "accuracy" degradation on the task I was training on.
Increasing rank does improve performance (we started with 8) but, I had found, has diminishing returns & results in marginal gains vs the others levers. Alpha, in that case, was kept static as per the paper (iirc) and we found following heuristic of alpha = 2*rank wasn't particularly helpful in moving the needle.
Might be useful, I had posted this thread that has articles I found helpful (even though, some were published after & replicated my private findings).

@lucapericlp i think we were wondering if it’s worth integrating with PEFT or is the current way of doing things fine?

I'd say using PEFT is preferable purely from standpoint of reducing codebase footprint for "standard" ops where possible.

@danablend any news?


Upgrade versions of some dependencies

Now that our MSRV is 1.65, it unlocks upgrading some dependencies' versions.

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 87.34%. Comparing base (2c2e1b3) to head (6c89795).

Additional details and impacted files
@@                 Coverage Diff                  @@
##           validate-cast-const    #1886   +/-   ##
====================================================
  Coverage                87.34%   87.34%
====================================================
  Files                       16       16
  Lines                     5964     5964
====================================================
  Hits                      5209     5209
  Misses                     755      755

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


Create China Trade Visualization Objects.

China Trade:
Which of the top 20 trading nations are least dependent on trade with China?
Is the a correlation between increased trade with China and increased GDP with China’s trading partner nations?

Completed. ...


-f best not downloading the best

*I searched all open issues for -f best and found no issues that reported this issue directly, but many issues that included the flag in their command. Issue #22988 is the opposite of this issue.
Checklist

[ ] I'm reporting a broken site support issue
[x] I've verified that I'm running youtube-dl version 2019.12.25
[x] I've checked that all provided URLs are alive and playable in a browser
[x] I've checked that all URLs and arguments with special characters are properly quoted or escaped
[x] I've searched the bugtracker for similar bug reports including closed ones
[x] I've read bugs section in FAQ

Verbose log

youtube-dl.exe -v -f best "https://www.youtube.com/watch?v=yfyhcnF7y1o"
[debug] System config: []
[debug] User config: []
[debug] Custom config: []
[debug] Command-line args: ['-v', '-f', 'best', 'https://www.youtube.com/watch?v=yfyhcnF7y1o']
[debug] Encodings: locale cp1252, fs mbcs, out cp437, pref cp1252
[debug] youtube-dl version 2019.12.25
[debug] Python version 3.4.4 (CPython) - Windows-10-10.0.18362
[debug] exe versions: ffmpeg git-2019-10-21-a174e5f, ffprobe git-2019-10-21-a174e5f
[debug] Proxy map: {}
[youtube] yfyhcnF7y1o: Downloading webpage
[youtube] yfyhcnF7y1o: Downloading video info webpage
[debug] Invoking downloader on 'https://r2---sn-hp57yn7r.googlevideo.com/videoplayback?expire=1577481627&ei=OyEGXrbuHcqs4QTgsKWYAQ&ip=2600%3A1700%3Adda1%3A3fa0%3A3da7%3Aabf6%3Ac1d1%3A5323&id=o-AD2hFJwD9RG4FRlenA1m69GA8eqZyCHcnEGpiwYXkqy8&itag=22&source=youtube&requiressl=yes&mm=31%2C26&mn=sn-hp57yn7r%2Csn-5ualdnl7&ms=au%2Conr&mv=m&mvi=1&pl=43&initcwndbps=1436250&mime=video%2Fmp4&ratebypass=yes&dur=571.234&lmt=1577352685493967&mt=1577459949&fvip=2&fexp=23842630&c=WEB&txp=5532232&sparams=expire%2Cei%2Cip%2Cid%2Citag%2Csource%2Crequiressl%2Cmime%2Cratebypass%2Cdur%2Clmt&sig=ALgxI2wwRAIgWCnliV8wLAXRVUlKj6rAXxDSbtTZznkXhYu3lXdVCg0CIEWX9GMuJL2cVEUg4UXnP2AJ8pusUHYuOzvd1fKhnqS0&lsparams=mm%2Cmn%2Cms%2Cmv%2Cmvi%2Cpl%2Cinitcwndbps&lsig=AHylml4wRQIgD7CXEM2bm7htDgYcyXQK60AQM362mCEAU5zaWEnVGxMCIQDbyTYmt4uBKgON30O2PDB8zTtihHFlZewyMkNhqWnedg%3D%3D'
[download] Destination: Beijing 360º - Tiananmen Square and the Forbidden City-yfyhcnF7y1o.mp4
[download] 100% of 163.43MiB in 01:27

Description

Attempting to download a YouTube video with 5K (2880) output, but it only downloads the 720 version.
youtube-dl.exe -v -F "https://www.youtube.com/watch?v=yfyhcnF7y1o"
[debug] System config: []
[debug] User config: []
[debug] Custom config: []
[debug] Command-line args: ['-v', '-F', 'https://www.youtube.com/watch?v=yfyhcnF7y1o']
[debug] Encodings: locale cp1252, fs mbcs, out cp437, pref cp1252
[debug] youtube-dl version 2019.12.25
[debug] Python version 3.4.4 (CPython) - Windows-10-10.0.18362
[debug] exe versions: ffmpeg git-2019-10-21-a174e5f, ffprobe git-2019-10-21-a174e5f
[debug] Proxy map: {}
[youtube] yfyhcnF7y1o: Downloading webpage
[youtube] yfyhcnF7y1o: Downloading video info webpage
[info] Available formats for yfyhcnF7y1o:
format code  extension  resolution note
249          webm       audio only tiny   52k , opus @ 50k (48000Hz), 3.16MiB
250          webm       audio only tiny   68k , opus @ 70k (48000Hz), 4.16MiB
140          m4a        audio only tiny  130k , m4a_dash container, mp4a.40.2@128k (44100Hz), 8.82MiB
251          webm       audio only tiny  133k , opus @160k (48000Hz), 8.35MiB
278          webm       256x144    144s  105k , webm container, vp9, 30fps, video only, 6.58MiB
160          mp4        256x144    144s  117k , avc1.4d400c, 30fps, video only, 7.50MiB
242          webm       424x240    240s  230k , vp9, 30fps, video only, 15.03MiB
133          mp4        424x240    240s  249k , avc1.4d4015, 30fps, video only, 16.31MiB
243          webm       640x360    360s  478k , vp9, 30fps, video only, 28.73MiB
134          mp4        640x360    360s  637k , avc1.4d401e, 30fps, video only, 42.65MiB
244          webm       852x480    480s  817k , vp9, 30fps, video only, 51.95MiB
135          mp4        852x480    480s 1154k , avc1.4d401f, 30fps, video only, 78.18MiB
247          webm       1280x720   720s 1863k , vp9, 30fps, video only, 106.98MiB
136          mp4        1280x720   720s 2313k , avc1.4d401f, 30fps, video only, 156.36MiB
248          webm       1920x1080  1080s 3578k , vp9, 30fps, video only, 193.54MiB
137          mp4        1920x1080  1080s 4333k , avc1.640028, 30fps, video only, 292.37MiB
271          webm       2560x1440  1440s 9144k , vp9, 30fps, video only, 601.10MiB
264          mp4        2560x1440  1440s 10400k , avc1.640032, 30fps, video only, 697.57MiB
313          webm       3840x2160  2160s 17919k , vp9, 30fps, video only, 1.18GiB
272          webm       5760x2880  2880s 19910k , vp9, 30fps, video only, 950.76MiB
266          mp4        3840x2160  2160s 23210k , avc1.640033, 30fps, video only, 1.50GiB
138          mp4        5760x2880  2880s 39271k , avc1.64003c, 30fps, video only, 2.56GiB
43           webm       640x360    360s , vp8.0, vorbis@128k, 73.26MiB
18           mp4        640x360    360s  781k , avc1.42001E, mp4a.40.2@ 96k (44100Hz), 53.20MiB
22           mp4        1280x720   720s 2400k , avc1.64001F, mp4a.40.2@192k (44100Hz) (best)

read https://github.com/ytdl-org/youtube-dl#format-selection.

@remitamine The documentation isn't as clear as it could be. I had ffmpeg on my PATH, but youtube-dl needed the binaries in the same directory to properly download the best video option for YouTube. But, thank you.
I'll see if I can propose a documentation change that would make this clearer.

youtube-dl needed the binaries in the same directory to properly download the best video option

Wrong.


CloudStack cluster upgrade fails when using unstacked etcd

What happened:
Cluster upgrade was stuck on deleting the etcd VM with error
What you expected to happen:
Upgrade to succeed
How to reproduce it (as minimally and precisely as possible):
Perform an upgrade using an EKS-A cluster with CAPC v0.4.2 release with an unstacked etcd.
Anything else we need to know?:
This PR fixes the issue: https://github.com/aws/cluster-api-provider-cloudstack/pull/69. It will be addressed in the next CAPC release, so we'll need to upgrade when that is available
Environment:

EKS Anywhere Release:
EKS Distro Release:

@mitalipaygude This issue will be fixed with the next CAPC release in the upcoming eks-a release, it should not be postponed to next+1

resolved with CAPC version bump


Support protected directories

by setting the sticky bit to all explicitly requested directories.
Closes #236
This MR requires explicit requests for (protected) directories in the update file system route.

My todo: Also update the Docker image and ...

[ ] set the sticky bit for /workspace
[ ] reset the owner for /workspace to root

I rebased the branch (to include the latest changes from main) and thereby also triggered a new build including the changed Dockerfile from https://github.com/openHPI/dockerfiles/pull/24. Unfortunately, I had to change the Makefile as well, so that the changes in the base image are applied. See #243.


Fix Gradle runtime JDK version post 8.10 version upgrade

Description
Fix Gradle runtime JDK version post 8.10 version upgrade
Related Issues
Related to https://github.com/opensearch-project/OpenSearch/pull/15177

Check List

[ ] Functionality includes testing.
[ ] API changes companion pull request created, if applicable.
[ ] Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@dblock no need to backprort, this is a fix to 2.x already, thanks a lot!


Can't find 0.world.gzip file

Hi!
I try to use your docker to set up a dedicated server on my centos 7.6 server.
I followed your instructions and successfully use your example docker -run xxxx command to run the docker file.
But I can't find the 0.world.gzip file you mentioned on my server  :(
I checked the path /var/lib/docker/volumes/corekeeper_save_volume/_data/DedicatedServer/,
But there is only one file named ServerConfig.json.
I also got these logs after running docker logs -f core-keeper-dedicated:
[ 99%] Downloading update (19,166 of 19,191 KB)...
[ 99%] Downloading update (19,191 of 19,191 KB)...
[100%] Download complete.
[----] Installing update...
[----] Extracting package...
[----] Extracting package...
[----] Extracting package...
[----] Extracting package...
[----] Installing update...
[----] Installing update...
[----] Installing update...
[----] Installing update...
[----] Installing update...
[----] Installing update...
[----] Installing update...
[----] Installing update...
[----] Cleaning up...
[----] Update complete, launching Steamcmd...
Redirecting stderr to '/home/steam/Steam/logs/stderr.txt'
[  0%] Checking for available updates...
[----] Verifying installation...
Steam Console Client (c) Valve Corporation - version<PHONE_NUMBER>
-- type 'quit' to exit --
Loading Steam API...OK
Connecting anonymously to Steam Public...OK
Waiting for client config...OK
Waiting for user info...OK
 Update state (0x3) reconfiguring, progress: 0.00 (0 / 0)
 Update state (0x3) reconfiguring, progress: 0.00 (0 / 0)
 Update state (0x3) reconfiguring, progress: 0.00 (0 / 0)
 Update state (0x61) downloading, progress: 8.45 (23701836 / 280515706)
 Update state (0x61) downloading, progress: 24.26 (68048285 / 280515706)
 Update state (0x61) downloading, progress: 38.87 (109022774 / 280515706)
 Update state (0x61) downloading, progress: 59.46 (166788718 / 280515706)
 Update state (0x61) downloading, progress: 87.22 (244667640 / 280515706)
 Update state (0x81) verifying update, progress: 63.56 (178291676 / 280515706)
Success! App '1963720' fully installed.
Started server process with pid 90
_XSERVTransmkdir: ERROR: euid != 0,directory /tmp/.X11-unix will not be created.
Game ID: (The game ID, it is same as the game ID in ServerConfig.json)

I also checked the running status by docker ps -a, and it shows:
CONTAINER ID        IMAGE                           COMMAND             CREATED             STATUS              PORTS               NAMES
3e7297f91cca        xalimar/core-keeper-dedicated   "bash entry.sh"     18 minutes ago      Up 18 minutes                           core-keeper-dedicated

I am a noob on docker and linux, please help me, thank you!
quq

Do I have to open certain ports on my server ?

Once the game is running, I believe you need to connect to it and bash a few walls and wait a few minutes to cause the server to create the world save file.

Once the game is running, I believe you need to connect to it and bash a few walls and wait a few minutes to cause the server to create the world save file.

Thanks !
I am able connect to use this game server now!


Correct quotation marks for Windows cmd.

For Windows users the provided example with single quotes doesn't work. Correct are double quotes for Windows cmd.
We spent almost hour finding out why our protractor parameters works on my machine and does not on colleague PC :-/

I signed it!

I signed it!

Closed with bc16de7


to!string doesn't throw on invalid UTF sequence

andrej.mitrovich (@AndrejMitrovic) reported this on 2011-06-08T11:41:02Z
Transfered from https://issues.dlang.org/show_bug.cgi?id=6125
CC List

andrei (@andralex)
bugzilla (@WalterBright)
dlang (@Vild)
lucia.mcojocaru

Description
I'm not sure if this is a bug or wanted behavior:
    auto x = to!string(cast(char)255);

That won't throw. But this will:
    auto x = to!string(cast(char)255);  // or try 128
    auto z = toUTF8(x);  // throws

I've had this example code translated from C:

    foreach (y; 0 .. 16)
    foreach (x; 0 .. 16)
    {
        auto buffer = to!string(cast(char)(16 * x + y));
        auto result = buffer.toUTF16z;  // call to utf16z for the winapi
    }

Essentially the code builds a table of characters that it prints out. But it doesn't seem to take into account invalid UTF8 code points.

This leads me to another question, how does one iterate through valid UTF code points, starting from 0? Is there a Phobos function that does that?

andrej.mitrovich (@AndrejMitrovic) commented on 2016-08-27T21:55:57Z
-----
import std.conv;
import std.stdio;

void main()
{
    auto x = to!string(cast(char)255);
    writeln(x);
}
-----

Outputs:
[Decode error - output not utf-8]

I think the to!() routines should be UTF safe so the call to to!string above should throw an exception. Is this right Andrei?

lucia.mcojocaru commented on 2016-11-21T13:36:26Z
Is this a Windows specific bug?

I tested the following on Linux 64:
  1 import std.conv;
  2 import std.stdio;
  3 import std.utf;
  4
  5 void main()
  6 {
  7     auto x = to!string(cast(char)191);
  8     auto z = toUTF8(x);
  9     writeln(x);
 10
 11
 12     foreach (y; 0 .. 16)
 13         foreach (r; 0 .. 16)
 14         {
 15             auto buffer = to!string(cast(char)(16 * r + y));
 16             auto b = toUTF8(buffer);
 17             writeln(b);
 18 //            auto result = buffer.toUTF16z;  // call to utf16z for the winapi
 19         }
 20 }

Only the commented line throws:
core.exception.UnicodeException@src/rt/util/utf.d(292): invalid UTF-8 sequence

andrei (@andralex) commented on 2016-10-14T16:55:25Z
Well since it doesn't throw we may as well make it nothrow :o) and use the replacement char, or add an overload. I'll bootcamp this.

bugzilla (@WalterBright) commented on 2019-12-11T14:18:40Z
The original bug isn't windows specific. I don't know if the example from Lucia Cojocaru can be considered the same bug...


Tray icons turn invisible after a few hours

I've noticed an issue in which tray icons start turning invisible after running PingoMeter for a few hours (configuration below).
Screenshot:

I can still interact with them (hover, left and right click), but the icon remains invisible. This continues even after closing PingoMeter and has only ever happened when PingoMeter was running (which also seems to be the first icon to turn invisible). Icons reappear after a reboot.
Here is my config.txt:
# PingoMeter config file
Delay 1000
MaxPing 100
BgColor 0:0:0
GoodColor 120:180:0
NormalColor 255:180:0
BadColor 255:0:0
RunOnStartup False
TheIPAddress <IP_ADDRESS>
AlarmConnectionLost True
AlarmTimeOut True
AlarmResumed True
UseNumbers False
SFXConnectionLost C:\Windows\Media\Windows Pop-up Blocked.wav
SFXTimeOut (none)
SFXResumed (none)

I was able to observe this behavior on two seperate machines running the same environment (Windows 10, version 20H2).

Since it appears that this problem may be caused by another, unrelated application, I'm closing this issue until further information comes to light.

Since it appears that this problem may be caused by another, unrelated application, I'm closing this issue until further information comes to light.


Object destructuring fails if defaults are set

TypeScript Version:  2.2.1 / nightly (2.2.0-dev.201xxxxx)
Try it yourself in the playground
Code
function fancyMethod({ option1:boolean = true, option2: number = 1 } = {}) {
    console.log(option1, option2);
}

fancyMethod();
fancyMethod({option2: 3});

Expected behavior:
Console log shows value of option1 and option 2
Actual behavior:
Typescript shows errors:

Cannot find name 'option1'.
Cannot find name 'option2'.

I think this is what you want. Object destructuring with type sometime really confuses people.
function fancyMethod({ option1 = true, option2 = 1 }: { option1?: boolean, option2?: number } = {}) {
    console.log(option1, option2);
}
fancyMethod();
fancyMethod({option2: 3});

Thanks I tried this but didn't use it because of https://github.com/Microsoft/TypeScript/issues/16354
The solution now is:
function fancyMethod({ option1 = true, option2 = 1 }: object & { option1?: boolean, option2?: number } = {}) {
    console.log(option1, option2);
}

Which is a lot harder to read than
function fancyMethod({ option1:boolean = true, option2: number = 1 } = {}) {
    console.log(option1, option2);
}

This is not a valid TS code, since : in destructuring is considered renaming.
function fancyMethod({ option1: boolean = true, option2: number = 1 } = {}) { // <--
    console.log(option1, option2);
}

Additionally, {} is not assignable to { option1, option2 }, since there is no option1 and option2 in {}.

I think you can use type or interface to simplify the type declaration. For example:
interface X {
  option1?: boolean;
  option2?: number;
}
type NonPrimitive<T> = object & T;

function fancyMethod(x: NonPrimitive<X> = {}) {
  const {
    option1 = true,
    option2 = 1,
  } = x;
  console.log(option1, option2);
}

Unfortunately the interface would be far away from the method as it is a class method.
But thanks to your explanation with the renaming I am now understanding the reason behind it  👍


Add Instantaneous and Queued TrajectoryExecutors

This is a better implementation than #251, following @mkoval's suggestions on that pull request. This PR also makes various changes to the KinematicSimulation and Ros TrajectoryExecutors implementations to make them more consistent.

Before creating a pull request

[x] Document new methods and classes
[x] Format code with make format

Before merging a pull request

[x] Set version target by selecting a milestone on the right side
[ ] Summarize this change in CHANGELOG.md
[x] Add unit test(s) for this change

This almost works when I'm running some scripts in simulation. I'm running into issues while using QueuedTrajectoryExecutor to wrap KinematicSimulationTrajectoryExecutor.
The problem is that KinematicSimulationTrajectoryExecutor::execute attempts to lock the meta-skeleton that it operates on here. For some reason, it's unable to acquire the lock when it's being wrapped by QueuedTrajectoryExecutor, although it works just fine when it's not being wrapped. I'm looking for other places where I might be neglecting to unlock a locked meta-skeleton (especially in QueuedTrajectoryExecutor), but no luck yet.
The actual queuing logic seems fine; if I comment out the skeleton locking/unlocking, the wrapped KinematicSimulationTrajectoryExecutor works.

Ack, just figured out what was happening. In libherb, we lock the entire robot meta-skeleton before calling QueuedTrajectoryExecutor::step. I'm calling the wrapped TrajectoryExecutor::execute from QueuedTrajectoryExecutor::step, which expects the arm meta-skeleton to be unlocked as it checks the dof names. I'll think about how to fix this.

@mkoval When you get a chance, would you mind looking over my proposed fix for this locking issue in https://github.com/personalrobotics/aikido/pull/259/commits/06672e5ba52bb5458dd388696aa813e9fe6070a9?
The short version is that the DOF validation that is performed by [Instantaneous|KinematicSimulation]TrajectoryExecutor::execute requires the robot Skeleton to be locked. However, execute is called from QueuedTrajectoryExecutor::step, which is called while the robot Skeleton is locked.
Instead, I'm now performing the DOF validation in QueuedTrajectoryExecutor::execute, rather than QueuedTrajectoryExecutor::step. Then, by the time the trajectory is actually sent to the underlying executor in step, it no longer needs to be validated and doesn't need to acquire the Skeleton lock.

Thanks for the comments, @mkoval! :heart:

In almost all cases, we should use MAGI to handle this queuing instead of TrajectoryExecutor.
The best reason to add queuing to the TrajectoryExecutor API is to support trajectory queuing and preemption in hardware drivers - which this PR doesn't support.

Agreed. I'm not sure whether these changes will be useful, but I figured since we already had a somewhat-working version of the code that I'd clean it up in case it's a good jumping-off point for something else in the future. I've added QueuedTrajectoryExecutor::abort, but it seems difficult to actually abort an in-progress trajectory so this only aborts queued trajectories that haven't begun execution.

What is the purpose of the trajectory tag added in this PR?

This stemmed out of a conversation with @siddhss5 where he mentioned that we should have additional metadata associated with our trajectories (like we did in prpy) about whether the trajectory was smooth, constrained, which planner was used, etc.
Adding this particular piece of metadata was just so that I could avoid adding an argument to TrajectoryExecutor::execute that decided whether or not the trajectory needed to be validated (again, to avoid the deadlock situation). It's not a particularly good reason :sweat:

I'm not sure whether these changes will be useful, but I figured since we already had a somewhat-working version of the code that I'd clean it up in case it's a good jumping-off point for something else in the future.

I agree that this is potentially useful once we have more control over trajectory execution on real hardware in the future (cancelling, preempting, etc). 👍

I've added QueuedTrajectoryExecutor::abort(), but it seems difficult to actually abort an in-progress trajectory so this only aborts queued trajectories that haven't begun execution.

If you make abort() part of the generic TrajectoryExecutor API, then QueuedTrajectoryExecutor could call abort() on the underlying trajectory executor. This seems pretty straightforward for KinematicTrajectoryExecutor, since we could set a flag that interrupts the next iteration of the simulation loop.
We can eventually support this on RosTrajectoryExecutor by canceling the actionlib goal, once we support that on the ReWD controller side. That's probably outside the scope of this pull request. 🙃

Adding this particular piece of metadata was just so that I could avoid adding an argument to TrajectoryExecutor::execute() that decided whether or not the trajectory needed to be validated (again, to avoid the deadlock situation). It's not a particularly good reason 😓

Trajectory tags in general seem pretty useful for all the reasons that @siddhss5 gave. I'm not convinced that this is a great application of them, though. 😬 Unfortunately, I don't have a better solution to fixing the deadlock.

Thanks! I've added abort to the TrajectoryExecutor interface, and I'll merge this PR once Travis says we're good to go.

I'm not convinced that this is a great application of [trajectory tags], though. Unfortunately, I don't have a better solution to fixing the deadlock.

The options I've thought of are either (1) splitting up the validation like this [and either having this silly trajectory tag, or just moving this boolean from a trajectory tag to an execute argument], (2) locking the skeleton in Herb::step and passing the owned lock to the executors, (3) not locking the skeleton in Herb::step, or (4) not locking the skeleton in the executors. I might revisit this later and see how annoying (2) would be.

My original intent with the step() interface was (3). None of the step() functions should acquire locks internally. Instead, they should document their requirements (e.g. "requires the Skeleton passed to the constructor to be locked") and rely on the caller to acquire the lock. In the case of HERB, that would be Herb::step() in libherb. Would that solve the problem?

Not really, we're actually still following your original intent. The tricky part is writing QueuedTrajectoryExecutor::step so that it doesn't need to acquire any locks (since it calls TrajectoryExecutor::execute which makes no such promises). I'm generally in favor of the architecture that we have currently.


Introduce Typeface.Stretch and rework Typeface fallback selection

What does the pull request do?

This adjusts our Typeface fallback algorithm to match this behavior: https://drafts.csswg.org/css-fonts-4/#font-matching-algorithm
What is the current behavior?

What is the updated/expected behavior with this PR?

How was the solution implemented (if it's not obvious)?

Checklist

[ ] Added unit tests (if possible)?
[ ] Added XML documentation to any related classes?
[ ] Consider submitting a PR to https://github.com/AvaloniaUI/Documentation with user documentation

Breaking changes

Obsoletions / Deprecations

Fixed issues

Just tried this out and added:
<TextBlock FontStretch="UltraCondensed">
    Hello World!
</TextBlock>

To the MainWindow.xaml in Sandbox, but I don't see any difference. What am I missing?

This is expected because not all font stretches are supported. There is not a single font family that I know that supports all stretch values.
On Google fonts you can filter by font width to find a font that matches a specific stretch value.
https://fonts.google.com/?width=1
I think this is kind of outdated tech. Today there are variable fonts that support a greater variety of customization.

You can test this PR using the following package version. 0.10.999-cibuild0018908-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

You can test this PR using the following package version. 0.10.999-cibuild0018908-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]


Scripts rewrite (#71)

General scripts rewrite to make the codebase cleaner and more understandable (and therefore maintainable).
The merging of this PR closes #71.

I think this PR is mergeable even though genStaff and genSchedule are not translated yet. I'm holding off on merging for now because we should probably consider https://github.com/GunnWATT/watt/pull/73/commits/1e17b5d85c8239c3af9161534b492051c1933a02.

After some experimentation and TS quirks, I think it would be preferable to stick to TS wrapper files. See discussion pursuant to https://discord.com/channels/710932856251351111/710936824121655396/913236678158733323.

To clarify about the TS wrappers,

Pros (keep)
Cons (remove)

Stricter type checking (due to types in TS being interfaces, type checking a const declaration in a TS file makes sure the object is entirely the required type, while type checking an import only guarantees that all requested fields exist and doesn't error if extra fields are present).
Massive thousand-line "TypeScript files" cause misrepresentation in the repository's language distribution percentage.

If data were imported by multiple components or pages, typing the data source is easier than typing the import in each file.
Not really idiomatic; the clubs JSON isn't "code" and shouldn't be represented as a TS file.

genClubs, genStaff, and genAlternates could write to ../client/src/data directly without requiring manual copying  of generated data.


DNSRecord CR gets stuck in deletion when whole namespace is being deleted

DNSRecord CR gets stuck in deletion due to missing Secret to cloud provider (only tried with AWS) if whole namespace is marked for deletion. If namespace where both DNSRecord and aws-credentias Secret reside is marked for deletion, would it be possible to ensure that DNSRecord CR is removed first and Secret only after that?
Steps to reproduce:
Have AWS credentials with permissions for Route53.
oc new-project test
kubectl -n test create secret generic aws-credentials \
  --type=kuadrant.io/aws \
  --from-literal=AWS_ACCESS_KEY_ID=$AWS_ACCESS_KEY_ID \
  --from-literal=AWS_SECRET_ACCESS_KEY=$AWS_SECRET_ACCESS_KEY

Create DNSRecord with spec similar to:
spec:
  endpoints:
    - dnsName: '*.test.yourdomain.com'
      recordTTL: 60
      recordType: A
      targets:
        - <IP_ADDRESS>
  providerRef:
    name: aws-credentials
  rootHost: '*.test.yourdomain.com'

Wait for DNSRecord to be reconciled and related records to be created in AWS
oc delete ns test
The namespace deletion gets stuck due to DNSRecord being stuck in deletion due to missing aws-credentials Secret. From a few tries I did it seems that the records in AWS are actually removed so there do not seem to be any leftovers there. Just the DNSRecord CR being stuck.
See that records were actually removed in AWS.
Remove the DNSRecord CRs finalizer to unblock the namespace deletion.
I tried wtih AWS only.

this is a known issue, the current solution is to add statuses to the deleting items to point at that they cannot clean up the zone, we decided that removing the resources without cleaning the zone (and so not pointing this dirty zone out to the user) is worse than causing a permanent issue with deletion in order to bring the dirty zone to the attention of the user.
ping @maleck13 @mikenairn @maksymvavilov

Make sense, so it is users responsibility to make sure that DNSRecord is removed before the secret gets removed. Thanks!

Yes we will ensure this is called out in documentation also


SPARK-13052 waitingApps metric doesn't show the number of apps currently in the WAITING state

Can one of the admins verify this patch?

ok to test

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50502/
Test FAILed.

Merged build finished. Test FAILed.

@RaafatAkkad looks like Master#waitingApps no longer needs to be exposed. Could you make it a private val?

retest this please

Test build #50510 has started for PR 10959 at commit 9549c83.

Test build #50510 has finished for PR 10959 at commit 9549c83.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50510/
Test FAILed.

Merged build finished. Test FAILed.

retest this please

Test build #50759 has started for PR 10959 at commit 9549c83.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50766/
Test FAILed.

Merged build finished. Test FAILed.

retest this please

LGTM

Test build #50759 has finished for PR 10959 at commit 9549c83.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50759/
Test PASSed.

Thanks for the feedback. The tests have passed. Can you please re-trigger a CI build for the "Make waitingApps private" commit as it has failed?

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50768/
Test FAILed.

retest this please

Actually the tests passed but not for the latest commit. Let's re-run this.

Test build #50772 has started for PR 10959 at commit f040710.

Test build #50772 has finished for PR 10959 at commit f040710.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50772/
Test PASSed.

Merged into master.


Viewer crashes when closing w/o stopping recorded sequence

Required Info

SDK Version
development (2.14.1)

Load a recorded sequence in the viewer and while it's playing try to quit => crash (stopping the sequence before quitting doesn't)

This might be a bug (I think I saw it once, but not on all systems), we will have to look more closely into it.

[Realsense Customer Engineering Team Comment]
hi @ogoshen,
would like to confirm some basic for me to have a try.
what's your OS? using D415 or D435?
thanks.

Windows 10, happens with recordings on either cameras.

[Realsense Customer Engineering Team Comment]
hi @ogoshen,
recorded with D415 on win10, with realsense viewer from v2.15.0, then load the recorded sequence from viewer, play and click the "X" of the bag file and no issue, maybe issue fixed in v2.15, can you please confirm?

'X' to close the window causes the crash.
Closing the bag before quitting is fine.

[Realsense Customer Engineering Team Comment]
hi @ogoshen,
then it's no crash here, so would like to confirm if you are testing with v2.15?
if so, can you reproduce this issue everytime? or what's the frequency?
thanks.

Should be resolved in the upcoming release


Fix: Bug with filtering data & server side pagination

What kind of change does this PR introduce? (check one with "x")

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior? (You can also link to an open issue here)
issue https://github.com/swimlane/ngx-datatable/issues/515
associated issue from PR https://github.com/swimlane/ngx-datatable/pull/543
What is the new behavior?
If there is enough data to display, no change in offset is done. (cases: server side pagination or filtering)
If there are not enough rows to display, change offset to maximum value that can show table rows. (cases: filtering)
Does this PR introduce a breaking change? (check one with "x")

[ ] Yes
[x] No

I am now facing a simmilar issue when I want to use a facet to replace data which is in the table.
I'm finding that I want to go back to the first page after replacing my data. I have pulled down this PR but it doesn't quite work. I am going to see if I can come up with something that works for both of us.
I would maybe hold off on merging this until I have tomorrow morning to look at it if that is alright.

Yup you are right, that should still be a issue as this implementation assumes that you have your table with one set of data that can grow, or filtered but won't change in between.
Solution I can think of is instead of changing actual input (rows) from outside on some filter we can have inbuilt generic filters like of sorting. @deeg @amcdnl

I want to use a facet to replace data which is in the table

BTW if you are meaning multiple filters here, then I think this should work fine as this has nothing to do with implementation of filters, it just plays with the new data set passed.
If you still find something non-functional, please share I'll also try to find a rigid solution to this. @deeg

@ritikrishu, please see the discussion back on the issue page.

@deeg - Any progress on this?

@amcdnl, take a look at my comment here
I went with option 2 to fix it for myself, and just manually adjust the offset.
My advice would be for people to do the same, maybe making it easier by updating the offset when the offset input changes so they don't have to manually grab the DatatableComponent if they didn't want to.
I'm not sure this PR is needed. Thoughts?


EqualsAvoidsNull removes too many parentheses

The java EqualsAvoidsNull recipe removes parentheses around rewritten comparisons. However, for some reason it is way more agressive in removing parentheses than it should be. The following test fails, when added to org.openrewrite.java.cleanup.EqualsAvoidsNullTest.java
    @Test
    void keepNecessaryParens() {
        rewriteRun(
          java(
            """
            import java.util.Collection;

            public class EqualsAvoidsNullTestClass {
                public void triggersRecipe(String toCheck) {
                    if (toCheck != null && toCheck.equals("stringLiteral")) { }
                }

                public boolean triggersParenthesesRemoval() {
                    return (System.getProperties() != null) ? System.getProperties().keySet().isEmpty() : false;
                }

                public boolean needsToKeepParentheses() {
                    Collection<Object> set = System.getProperties().keySet();
                    return !(set == null || set.isEmpty());
                }
            }
            """,
            """
            import java.util.Collection;

            public class EqualsAvoidsNullTestClass {
                public void triggersRecipe(String toCheck) {
                    if ("stringLiteral".equals(toCheck)) { }
                }

                public boolean triggersParenthesesRemoval() {
                    return System.getProperties() != null ? System.getProperties().keySet().isEmpty() : false;
                }

                public boolean needsToKeepParentheses() {
                    Collection<Object> set = System.getProperties().keySet();
                    return !(set == null || set.isEmpty());
                }
            }
            """
          )
        );
    }

Thanks for the detailed report again!

My pleasure. I want to use OpenRewrite in my current modernization effort, but am hampered by having the results of applying the Java cleanup recipes on our codebase sometimes not even compilable.
Note that this test needs all three methods; removing either of the first two will not trigger the bug, as far a I've been able to determine.

Sorry to hear these imperfections are slowing you down, and thanks for reporting all of those in such detail!
Did you already know you can also easily create a subset of recipes to run through our builder, even for local runs?
You can for instance start at this composite recipe:
https://public.moderne.io/recipes/org.openrewrite.java.cleanup.CommonStaticAnalysis
From there (de)select any that you want to run, then press "Add selected to builder".

That then brings you to our yaml recipe builder, where you can copy the yaml recipe to a local rewrite.yml fiile.
type: specs.openrewrite.org/v1beta/recipe
name: org.timtebeek.MyCustomRecipe
displayName: My custom recipe
description: Fix all the things.
recipeList:
  - org.openrewrite.java.cleanup.DefaultComesLast
  - org.openrewrite.java.cleanup.EmptyBlock
  - org.openrewrite.java.format.EmptyNewlineAtEndOfFile
  - org.openrewrite.java.cleanup.ForLoopControlVariablePostfixOperators
  - org.openrewrite.java.cleanup.FinalizePrivateFields
  - org.openrewrite.java.cleanup.MethodParamPad

This rewrite.yml can then be used directly in your local runs with the instructions here:
https://docs.openrewrite.org/running-recipes/running-rewrite-on-a-maven-project-without-modifying-the-build#running-a-recipe-with-configuration-parameters
Hope that at least helps you to run a large selection of recipes more easily, minus the recipes you want to exclude for now due to the issues reported. And always happy to help you on our Slack channel as well.

Oh, thanks! The UI is nice. Although I had already built a rewrite.yml file for my own purposes (initially to exclude the Rename..ToCamelCase recipes), this is helpful.

Hi, I would like to attempt a solution here. Tho, I probably need time by next weekend, loads of work these days.
As far as I understood, the UnwrapParentheses class should not evaluate all Unary-Operations as unwrappable. For PreIncrement, PreDecrement, PostIncrement, and PostDecrement it should always be unwrappable because only the direct use of variables is allowed. Tho, for Positive, Negative, Complement and also Not, we would need another check if there is a complex term inside the parentheses.
In the provided testcase by @arthurvl , thank you for that one, it is the latter case of Not which should be evaluated as non unwrappable due to the operation inside the parentheses.
Kind regards
Alexander Skrock


Wrong component name for folders with dash (-)

Describe the bug
When nested component's parent folder has dash (-) it gives off a wrong component name.
RoutedMessagesIdePanel: typeof import('./src/components/routed-messages/RoutedMessagesSidePanel.vue')['default']

Right now, changing the parent folder to routedMessages works
Reproduction
https://stackblitz.com/edit/vitejs-vite-vtvczj?file=components.d.ts
System Info
System:
    OS: Windows 11 10.0.22621
    CPU: (12) x64 AMD Ryzen 5 3600 6-Core Processor
    Memory: 12.74 GB / 31.95 GB
  Binaries:
    Node: 16.18.1 - C:\Program Files\nodejs\node.EXE
    npm: 8.19.2 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Chromium (121.0.2277.128)
    Internet Explorer: 11.0.22621.1

Used Package Manager
npm
Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guide.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Check that this is a concrete bug. For Q&A, please open a GitHub Discussion instead.
[X] The provided reproduction is a minimal reproducible of the bug.

This is because you set collapseSamePrefixes: true, which collapses the same prefixes to Ide.
See the definition.


Mapping guide edits

Edits made to main branch parsers & schemas after mapping guide review.
Changes tracked in #146
TODO: move Guide edits to mapping-guide branch before merging to main (or just merge the mapping guide and edit it on main - could get messy though.)

I'm wondering regarding sex/gender and sex at birth whether these should be made separate attributes? where sex at birth has explicitly been collected this could be different to current gender, which was not collected. therefore combining sex at birth and gender could be combining quite different sets of data. Users could use either sex at birth or gender but it would be clear what the data mean if kept separate

I'm wondering regarding sex/gender and sex at birth whether these should be made separate attributes? where sex at birth has explicitly been collected this could be different to current gender, which was not collected. therefore combining sex at birth and gender could be combining quite different sets of data. Users could use either sex at birth or gender but it would be clear what the data mean if kept separate

We had a discussion about this last week, and initially decided to raise it as an issue, because we were getting a mix of sex & gender (as you say) in the sex_at_birth attribute which is clearly flawed. Having gone back through the current parsers, it appears a lot of that was just mapping errors (not known -> non-binary for example), and having fixed those there appears to be less of a problem.
However as you say there are a couple of studies which recorded sex rather than sex_at_birth, and in some of those cases options like 'transgender' are given in the data dictionary (although they don't appear in the data). I'd be open to making a separate field for those studies that don't explicitly state sex at birth, calling just 'sex' or 'sex_gender' or similar to separate it out, and making a requirement that one of them must be filled for the data to be valid.

very good!
what did we decide on "renal_replacement_therapy_dialysis"? it is in the observation schema, but dialysis is also a treatment in the visits table. I think the suggestion was to remove "renal_replacement_therapy_dialysis" from the observations table? ..

This has been removed in a couple of pending pull requests, but I've done it here too just for clarity.


Fix the rosdep_repo_check automation in CI

It seems that 'HEAD^' isn't usable in GitHub Actions for some reason. That isn't really a problem - we can mimic the same behavior by excluding the target ref entirely.
Invoking the test locally will now inspect only unstaged changes, where invoking the test in a GitHub PR will inspect incoming changes. During GitHub CI, no changes will be inspected.
Follow-up to #28902

Invoking the test locally will now inspect only unstaged changes

I'm actually pretty into this as default behavior.
I wanted to add how to run on uncommitted, unstaged changes to the README and now it's the default \o/


Disable "lookup" option on player right click

There is currently no option in the settings to disable the hiscores right click lookup function.

If you don't want hiscore lookup stuff at all (no sidebar, no right click), the next release (which should go live on thursday) will correctly remove the option from the right click menus, thanks to ae1eaef6

Ah.... Thank you

Fixed in 07addf681a4af276f8bcf979b3103e2a5f2a6894


Completing HTTP methods

I figured while I was looking at this, I would go ahead and round out the request methods available from the python requests module into Scales. Feel free to take or decline this PR - just figured I could save you a little work if you'd like these included,

lgtm, thanks again for all the contributions!


Exception: The model predicted the custom action 'action_check_status' but you didn't configure an endpoint to run this custom action.

Rasa Core version:
rasa-core==0.11.11
rasa-core-sdk==0.11.5
rasa-nlu==0.13.6
Python version:  3.6.3
Operating system (windows, osx, ...): Mac OSX 10.11.16
WARNING:rasa_core.agent:Passing a text to `agent.handle_message(...)` is deprecated. Rather use `agent.handle_text(...)`.
ERROR:rasa_core.processor:Encountered an exception while running action 'action_check_status'. Bot will continue, but the actions events are lost. Make sure to fix the exception in your custom code.
ERROR:rasa_core.processor:The model predicted the custom action 'action_check_status' but you didn't configure an endpoint to run this custom action. Please take a look at the docs and set an endpoint configuration. https://rasa.com/docs/core/customactions/
Traceback (most recent call last):
  File "/Users/DHarun/anaconda3/envs/x/lib/python3.6/site-packages/rasa_core/processor.py", line 338, in _run_action
    events = action.run(dispatcher, tracker, self.domain)
  File "/Users/DHarun/anaconda3/envs/x/lib/python3.6/site-packages/rasa_core/actions/action.py", line 305, in run
    "".format(self.name(), DOCS_BASE_URL))
Exception: The model predicted the custom action 'action_check_status' but you didn't configure an endpoint to run this custom action. Please take a look at the docs and set an endpoint configuration. https://rasa.com/docs/core/customactions/

Content of domain file (if used & relevant):
endpoints.yml
action_endpoint:
url: "http://localhost:5055/webhook"

Info about server for custom actions:
$ python -m rasa_core_sdk.endpoint --actions actions
INFO:__main__:Starting action endpoint server...
INFO:rasa_core_sdk.executor:Registered function for 'action_check_status'.
INFO:__main__:Action endpoint is up and running. on ('<IP_ADDRESS>', 5055)

I use to develop rasa chatbot the anaconda environment.
I have a virtuell environment form the anaconda nagivator, I create from this two terminals:
first, start the server and the second to start the bot:
bot.py --endpoints endpoints.yml
the code for the bot:
import logging,  warnings
from rasa_nlu.training_data import load_data
from rasa_nlu.config import RasaNLUModelConfig
from rasa_nlu.model import Trainer
from rasa_core.policies.keras_policy import KerasPolicy
from rasa_core.agent import Agent

logging.basicConfig(level="INFO")
warnings.filterwarnings('ignore')

agent = Agent('domain.yml', policies=[KerasPolicy()])
training_data = agent.load_data('stories.md')
agent.train(
        training_data,
        validation_split=0.0,
        epochs=400
)

agent.persist('models/dialogue')

training_data = load_data('nlu.md')
pipeline = [{"name": "nlp_spacy"},
            {"name": "tokenizer_spacy"},
            {"name": "intent_featurizer_spacy"},
            {"name": "intent_classifier_sklearn"}]

trainer = Trainer(RasaNLUModelConfig({"pipeline": pipeline}))
interpreter = trainer.train(training_data)

model_directory = trainer.persist('./projects/default/')
agent = Agent.load('models/dialogue', interpreter=model_directory)

print("Your bot is ready to talk! Type your messages here or send 'stop'")
while True:
    a = input()
    if a == 'stop':
        break
    responses = agent.handle_message(a)
    for response in responses:
        print(response["text"])

I cant start the custom actions, how can I solve this issue? Thanks.

Error : The model predicted the custom action 'basic_info', but you didn't configure an endpoint to run this custom action. Please take a look at the docs and set an endpoint configuration via the --endpoints flag. https://rasa.com/docs/rasa/core/actions. I have created a basic form in actions.py