WIP: Reduce amount of std::map lookups performed during a sim

This PR covers reducing the amount of runtime lookups done by the various models, muscles, etc. in OpenSim at runtime.
Overview
Runtime std::map lookups can happen a lot in a simulation. The reason is probably because many concrete simulation/model implementations derive from Component, which has its own storage model for storing + caching things in the model graph. Derived classes are incentivized to perform lookups using the base Component class, rather than handling lookups themselves, because the code + datastructures are already there, so it's more convenient than caching the lookup indexes locally etc.
An example of where this happens: a class may want runtime access to some cached variable of type T in the current model being simulated. To do that, a common pattern is to call something like this->getModel().getCacheVariableValue<T>(st, "foo");. This ultimately breaks down into:

Get the current model (a pointer lookup + dereference)
Lookup the cache variable in a Component::_namedCacheVariableInfo, which is a std::map
Get the CacheEntryIndex from the CacheInfo held in the std::map
Get the SimTK::System the Component is attached to
Use the CacheEntryIndex to get the cache entry from the SimTK::System
Dereference the resulting entry
Downcast the entry to T with a dynamic_cast

Ignoring the details around pointer types, memory layout, etc. cycle estimation (valgrind) measurements on (e.g.) ToyDropLanding show results like:

Function Name
Called #
Incl. %
Self %

isCacheVariableValid
12 686 729
5.24
1.26

setCacheVariableValue
5 589 111
2.28
0.58

getCacheVariableValue
6 457 708
2.13
0.58

markCacheVariableValid
3 964 799
1.52
0.29

Not a massive deal--it's ultimately < 12 % of perf--but there might be some low-hanging fruit in terms of perf optimizations we can roll out to reduce the overhead of those functions. However, these higher-level lookup calls ultimately result in the profiler measuring a bunch of overhead in  related, lower-level, operations (string storage, map lookups, downcasting):

Function Name
Called #
Incl. %
Self %

std::_Rb_tree<...CacheInfo...> (lookups)
39 008 221
10.42
5.23

std::basic_string (ctor, key copies?)
58 939 682
3.75
2.36

__dynamic_cast (downcasts)
51 315 491
3.57
1.28

Other Points

@tgeijten has already done a bunch of legwork on this: https://github.com/tgeijten/opensim32/commit/56f825c4d014826ee073d4025a257ce6e58ebd44 . I separately measured similar perf problems before I knew about this.

Additionally, I performed extra measurements (callgrind simulations) and looked into theories around cache misses etc. Looking into it, the biggest source of L1 misses:

~3.1 %: Rb_tree  lookups
~2.9 %: Property<double>::getValue(int)
~2 %: realizeMeasureTimeVirtual(SimTK::State&)

Which is to say that there's nothing obviously evicting and reading L1 in one localized place (e.g. a loop that hits a large array of data, so all L1 fetches happen in that function). The L1 profile indicates that L1 misses are happening in many places all over OpenSim, which is probably because there's a huge amount of method nesting, getter nesting, abstraction, etc. that happens all over the place, rather than there being one obvious loop somewhere.

L3 misses:

30 % _dl_relocate_object
24 % _dl_lookup_symbol_x
7 % do_lookup_x

Which indicates that the application has to fetch its functions from main memory, probably at loadup. Assuming that, once the application is loaded, it spends most of its time in L1-L3. Other functions in the L3 simulation all look like object destructors etc. (also unsurprising).

To confirm this, I ran valgrind --tool=massif on the application, which recorded that peak heap usage during the simulation is ~110 MB, with 80 MB being mmaped (probably libraries, which will page-miss, and L3 miss, during the instruction fetch), 15 MB being from operator new (probably OpenSim/Simbody) and 8 MB being from malloc. My CPU (Ryzen 1600) has 16 MB of L3, and all of the memory will not be used in the core loop, so it's possible that OpenSim can reside entirely in L3 for the ToyDropLanding sim.

ToDo (from conculsions)

Almost all perf hits are for cache variable lookups. There are other std::maps in Component, but the biggest perf gains for the smallest amount of work should focus on just optimizing any of the (get|upd|set)*Cache* methods

So, ignoring the merge bs, etc. d4bb192 is essentially just a cleaned-up version of the original code that also optimizes out a few unnecessary copies during CacheEntry initialization. Passes tests and nothing particularly noteworthy in terms of perf:
repeats = 32

Test Name
master [secs]
σ [secs]
branch [secs]
σ [secs]
Speedup

Gait2354
0.26
0.00
0.26
0.00
1.00

ToyDropLanding
15.18
0.04
15.12
0.04
1.00

ToyDropLanding_nomuscles
0.44
0.00
0.44
0.00
1.01

passive_dynamic
5.01
0.01
5.00
0.02
1.00

passive_dynamic_noanalysis
3.18
0.02
3.19
0.02
1.00

Arm26
0.31
0.00
0.30
0.00
1.01

RajagopalModel
16.59
0.14
16.02
0.05
1.04

The refactored version basically has no effect on perf overall. Any changes in speed are basically noise:
repeats = 64

Test Name
master [secs]
σ [secs]
branch [secs]
σ [secs]
Speedup

Gait2354
0.26
0.00
0.27
0.00
0.98

ToyDropLanding
15.38
0.10
15.60
0.08
0.99

ToyDropLanding_nomuscles
0.45
0.00
0.45
0.00
1.00

passive_dynamic
5.06
0.02
5.07
0.01
1.00

passive_dynamic_noanalysis
3.19
0.01
3.19
0.02
1.00

Arm26
0.31
0.00
0.31
0.00
1.03

RajagopalModel
16.23
0.07
16.25
0.14
1.00

Which is to be expected, because the sequence of operations in this refactor should be exactly the same as the pre-refactor.

So the next thing to do is decide on how to "skip" the CacheVariable lookup (an expensive std::map lookup), by providing callers with a direct handle to (e.g.) the underlying index.
This all sounds easy and sane, but it's actually a potential source of bugs. Such as:

Giving users a direct handle to the index: depends on whether it's been allocated into simbody yet, which depends on Component::extendRealizeTopology being called. Potential footgun if downstream classes try to cache it too early. This can be fixed by providing callers with a wrapper (e.g. CacheVariable) that ensures the index is initialized before allowing a caller to do an index-related operation. This is effectively fixed by the range check that already happens in CacheVariable.

Letting downstream classes store the handle: assuming they grab the handle at the correct time, or a correct wrapper is exposed (above), the next footgun is that the downstream class will want to store the resulting cached index locally. The risk here is that you can, logically speaking, end up with a situation where the downstream class owns a reference to a CacheVariable that has already been destructed by a call to _namedCacheVariables.clear() (as is done by Component::reset). That's basically a segfault/bizzare bug waiting to happen.

One way to get around this is to just "make sure they don't use the wrappers after a call to Component::reset(), but that convention also depends on downstream implementers also knowing that the std::map is also cleared by Component::clearConnections, Component::finalizeFromProperties, and Component::updateFromXMLNode.

Another way to get around this is to have the std::map store shared_ptr<CacheVariable>, so that the reference lifetime is extended beyond when the CacheVariables are cleared from the underlying std::map. This would remove the possibility of a segfault, but introduce the possibility of CacheVariables outliving their intended lifespan. To get around that, the reset() method should also reset all CacheVariable's index arguments to SimTK::NaNs or whatever, so that using the CacheVariable beyond its intended lifetime causes a runtime exception that prompts downstream implementors to fix their lifetime semantics

Letting users copy Model implementations: If a user copies/.clone()s a Model implementation then all of Model's member properties are also going to be copied - including the CacheVariables. Conventionally, the copied Model will call reset() or something to ensure that the copy performs a fresh initialization for SimTK etc. However, again, this is merely a convention. What we definitely don't want to happen is that the copy ends up with the ability to mutate the source's CacheVariables, because the copied instance blindly copied references to the source's CacheVariables. This is something that will currently be caught by the std::map lookup failing, but that failure will not happen if the lookup is skipped. Copying a Model implementation should, by default, reset all CacheVariable members to a senteniel (e.g. a NaN) so that using the copied CacheVariable without initializing it in the copy causes an appropriate explosion.

So the latest commit tries to introduce CacheVariable<T>, which is a type-safe wrapper around the underlying cache variable that will throw if it hasn't been allocated into SimTK yet. Users can request the CacheVariable<T> and then perform mutations, etc. on it. I've added some usage points into the codebase, and it seems like things are marginally faster (because the lookups have been reduced slightly, but not entirely eliminated):
repeats = 64

Test Name
master [secs]
σ [secs]
branch [secs]
σ [secs]
Speedup

Gait2354
0.27
0.01
0.27
0.00
1.02

ToyDropLanding
15.74
0.26
15.13
0.16
1.04

ToyDropLanding_nomuscles
0.47
0.00
0.46
0.00
1.00

passive_dynamic
5.35
0.02
5.29
0.03
1.01

passive_dynamic_noanalysis
3.35
0.01
3.33
0.02
1.01

Arm26
0.32
0.00
0.31
0.00
1.01

RajagopalModel
17.59
0.09
17.30
0.07
1.02

This is a kind of middle-ground to where we want things to be. In this approach, users can request the CacheVariable<T> locally and then use it. The "ideal" approach would be for the downstream classes to hold onto the CacheVariable<T> as class members, so that they can skip lookups altogether. However, this is difficult because:

I definitely don't want to support default initialization for CacheVariable<T> yet (it would need some if checks on the public API to ensure that it throws appropriately)
I definitely don't want to support copy initialization for CacheVariable<T> yet (it could lead to aliasing of the underlying variable)
I don't want to support half-initialized CacheVariable<T>s, but the creation of the cache variable (via addCacheVariable) has no access to the necessary SimTK::State.

Those two things could be dealt with by having a copy constructor that invalidates the copy (e.g. by nulling pointers then throwing whenever it's used) and by enforcing that CacheVariable<T>s are initialized using (e.g.) a constructor initializer.
Overall, this isn't an automatically easy thing to fix because the creation point for cache variables (addCacheVariable) is separate from the actual creation point (topology realization) and the destruction point is either whenever the underlying storage is cleared or the component destructs. Solutions suggesting just storing the SimTK:: indexes could work, but are also reliant on knowing the lifetime semantics (the existing index getter methods can return stale indices).

With a few tweaks, I've got a changeset that looks mostly logical in terms of structure, and I've refactored some usage points to demonstrate how this new CacheVariable<T> abstraction can be used by callers to "skip" having to look things up multiple times.
The biggest performance gains are going to be from designing the internals of CacheVariable<T> to also safely deal with _namedCacheVariables.clear() happening elsewhere (which should invalidate any handles passed out) and the CacheVariable<T> being copied into a different component (which should also "invalidate" the handle, to prevent bizzare aliasing bugs).

Perf measurements show mostly a little noise, but some minor speedups in the ToyDropLanding (which is the model that instigated this investigation, and might use some of the usage points I've optimized):
repeats = 64
|                  Test Name | master [secs] | σ [secs] | branch [secs] | σ [secs] | Speedup |
| -------------------------- | ------------- | -------- | ------------- | -------- | ------- |
|                   Gait2354 |          0.26 |     0.01 |          0.26 |     0.00 |    1.01 |
|             ToyDropLanding |         14.78 |     0.06 |         14.01 |     0.06 |    1.05 |
|   ToyDropLanding_nomuscles |          0.44 |     0.00 |          0.43 |     0.00 |    1.01 |
|            passive_dynamic |          4.87 |     0.01 |          4.90 |     0.01 |    0.99 |
| passive_dynamic_noanalysis |          3.09 |     0.01 |          3.11 |     0.01 |    0.99 |
|                      Arm26 |          0.30 |     0.00 |          0.29 |     0.00 |    1.02 |
|             RajagopalModel |         15.93 |     0.05 |         15.19 |     0.11 |    1.05 |

So this latest implementation seems to work and speeds things up:
repeats = 64

Test Name
master [secs]
σ [secs]
branch [secs]
σ [secs]
Speedup

Gait2354
0.26
0.00
0.26
0.00
1.00

ToyDropLanding
14.71
0.01
12.86
0.01
1.14

ToyDropLanding_nomuscles
0.43
0.00
0.43
0.00
1.00

passive_dynamic
4.87
0.00
4.86
0.00
1.00

passive_dynamic_noanalysis
3.07
0.00
3.06
0.00
1.00

Arm26
0.29
0.00
0.28
0.00
1.06

RajagopalModel
14.91
0.01
14.02
0.01
1.06

Missing full rollout (there's still some parts of the codebase using the "old" method) and testing, but it looks like a fairly solid speedup on my desktop, at least.

Finally, I figured out why OpenSim reliably segfaults when switching std::map to std::unordered_map: it's because some code relies on cache entries being allocated with allocateLazyCacheEntry deterministically.
When using a std::map, allocateLazyCacheEntry will always allocate the cache variables in the same order (alphabetical). This means that indices from one Component can be used to index into a copy without having to worry about whether the index changed during the copy.
std::unordered_map (obviously) has no ordering guarantees, so allocateLazyCacheEntry may (and, based on my investigations, will) be called in a different order, even if it's allocating cache entries for a direct copy of some Component. This means that indices from one Component definitely shouldn't be used to index into the copy because, yes, it'll be in bounds (the total number of entries in the same) but, no, it might point to a completely different value.
Some code indirectly relies on the indices being the same between copies. For example, testMomentArms.cpp:testMomentArmDefinitionForModel reliably segfaults on my desktop if I switch Component::_namedCacheVariables to a std::unordered_map because the implementation of that test keeps a copy of a model's SimTK::State followed by reinitializing a new SimTK::State that's used for other purposes - the segfault is almost certainly because those two states get muddled up at some point and some method nukes the wrong piece of memory because of it.
The solution is to just ensure that entries are allocated into SimTK in a deterministic order, even if the underlying container is non-ordered. In my case, I can keep using std::unordered_map at runtime if I collect + std::sort the keys at insertion-time.

Ok, so it's mostly ready, apart from the fact that I've spent a few hours now just trying to wrangle a basic test case together for the API.
I'd write a long post about why it's taking so long but let me just condense my frustration down into "SimTK and OpenSim have very particular initialization APIs that will crash, throw, and waste off a bunch of time if you don't use them in a very particular way".
I still haven't established why the tests still throw. Something to do with the realization stage. I'm in a dire situation: after flying into the lofty heights of this architecture I realize that I've taken insufficient oxygen. I went down the rabbit hole of SimTK::_Measure being used to trampoline into extendRealizeTopology, and SimTK's bizzare forward-declaring of loads of its system-level API, which even manages to confuse CLion's find usage method, and I met a talking rabbit and a dragon named Steve. It's probably best for me to take a break from this.

@adamkewley thanks for a thorough investigation and prototyping solutions. It makes for some clear decisions and discussions.

A 'SimTK::Stateand state vector and cache variable layout is only valid for theSystem` that created it after realizing the Topology and Model stages. Creating a new System (even from the same Model) and we should not be able to use the state in both Systems. It seems like we were doing that at it should not be allowed IMHO.

Because we permitted 1. I think we get the segfault when we use a cloned State on a new System. . As a rule, an assignment SimTK::State  s2 = s1, must invalidate all cache entries. You can use values from one State to update another, e.f s2.updY() = s1.getY();

I see two distinct choices. Continue to use runtime addCacheVariable  with a handle to the CacheVariable wrapper OR
exploit the CacheVariable to be used as a special member variable. I like prefer the member variable approach because it mirrors our handling of Properties, Inputs and Outputs. I agree it requires strict InvalideValue enforcement until the Topology is realized, but I think we could handle that nicely, and even give that error consistently.  In the Member variable case I would like to see several PRs: 1. to create, demonstrated and test the new CacheVariable<T> Member variable and its uses cases and 2) to systematically replace a few instances in an intensive cache using class (like Frame). and 3) replace all the remaining implementations. Or something like that.
Happy to chat about it too.!

@aseth1 the latest iteration is somewhere between a local CacheVariable<T> handle and a "special" member variable (it's just a normal member variable). Current implementation looks something like this:
class SomeComponent : public Component {
    // usual stuff

    void extendAddToSystem(SimTK::MultibodySystem& s) const override {
        Super::extendAddToSystem(system);
        this->activationCV = this->addCacheVariable("activation", getDefaultActivation(), SimTK::Stage::Topology);
    }

    double getActivation(const SimTK::State& s) const {
        return this->getCacheVariableValue(s, this->activationCV);
    }

    // default initializer: NaN with no key: runtime error if used at the wrong time
    // copy ctor: NaNs any cached index
    mutable CacheVariable<double> activationCV;
};

The advantage of this is that the API is basically identical (and compatible with, due to overrides) the old API. It just adds a performance + type-safety if used. Codebases can still use the old approach. They can even use this new approach interchangeably with the old approach (e.g. the variable is created and held as a CacheVariable<T> but other parts of the system can still get information via the legacy string lookup).
The Component API now looks like this:
// old
void addCacheVariable(const std::string& cacheVariableName, const T& variablePrototype, SimTK::Stage dependsOnStage) const;

// replaced with: API-compatible method that also:
// - Returns a CacheVariable<T> for cache speedups
// - Accepts a args as values, so that callers can rvalue-construct the argument
CacheVariable<T> addCacheVariable(std::string cacheVariableName, T variablePrototype, SimTK::Stage dependsOnStage) const;

// Methods that take a std::string key are "legacy" methods
SimTK::CacheEntryIndex getCacheVariableIndex(const std::string& name) const;
SimTK::CacheEntryIndex getCacheVariableIndex(const CacheVariableDetails& cv) const;
const T& getCacheVariableValue(const SimTK::State& state, const std::string& k) const;
const T& getCacheVariableValue(const SimTK::State& state, const CacheVariable<T>& k) const;
const T& setCacheVariableValue(const SimTK::State& state, const std::string& k, const T& value) const;
const T& setCacheVariableValue(const SimTK::State& state, const CacheVariable<T>& k, const T& value) const;
T& updCacheVariableValue(const SimTK::State& state, const std::string& k) const;
T& updCacheVariableValue(const SimTK::State& state, const CacheVariable<T>& k) const;
bool isCacheVariableValid(const SimTK::State& state, const std::string& k) const;
bool isCacheVariableValid(const SimTK::State& state, const CacheVariableDetails& k) const;
void markCacheVariableValid(const SimTK::State& state, const std::string& name) const;
void markCacheVariableValid(const SimTK::State& state, const CacheVariableDetails& name) const;
void markCacheVariableInvalid(const SimTK::State& state, const std::string& name) const;
void markCacheVariableInvalid(const SimTK::State& state, const CacheVariableDetails& name) const;

// previously: std::map<std::string, CacheVariableInfo>
mutable std::unordered_map<std::string, StoredCacheVariable> _namedCacheVariables;

@aseth1 , in this PR, all uses of the legacy API have been replaced with the proposed method, so essentially all of the implementation + rollout is done (+ passing existing tests). The only blocker is writing new tests (still haven't fixed this) and deciding on particular API semantics.
Implementing this in a similar way to the Object properties etc. is compatible with the implementation route I've taken and should just require defining an appropriate macro. For example, OpenSim_DECLARE_CACHE_VARIABLE(name, T, prototype, comment);, which would expand into:
CacheVariable<T> name;
// TODO: initialization via `addCacheVariable`
const T& get_name(SimTK::State& s) { return this->getCacheVariableValue(s, this->name); }
const T& set_name(SimTK::State& s, T v) { return this->setCacheVariableValue(s, this->name, std::move(v)); }
T& upd_name(SimTK::State& s) { return this->updCacheVariableValue(s, this->name); }
bool isValid_name(SimTK::State& s) { return this->isCacheVariableValid(s, this->name); }
void markValid_name(SimTK::State& s) { this->markCacheVariableValid(s, this->name); }
void markInvalid_name(SimTK::State& s) { this->markCacheVariableInvalid(s, this->name); }

Which would be compatible with the existing and new APIs, but provide similar semantics to the other parts of the design. This should enable gradual adoption of the feature.

Tests are now implemented for this feature, so I think it's best that we discuss any changes necessary (macro-izing it, rewording things, API changes, whatever), but the work is at least in roughly the right shape (it works, it speeds things up, etc.).

@tgeijten if you get a chance, could you give your thoughts on the proposed API change for this (above: "the latest iteration is somewhere between a local CacheVariable handle and a "special" member variable..."). Things are basically implemented now on this PR, so the only remaining step is to kick the API into shape.

Looks like good strategy. Is CacheVariable is a new class? I can't find it anywhere.

@tgeijten yeah, it's defined in Component.h for now, and should maybe be moved into it's own implementation file, but it's essentially this:
/**
 * Type-erased interface for a CacheVariable<T>.
 *
 * - This is necessary for methods/containers that cannot be templated, or
 *   need to store a generic collection over many types.
 */
class CacheVariableDetails {
private:
    std::string name;
    mutable SimTK::ResetOnCopy<SimTK::CacheEntryIndex> maybeUninitIndex{SimTK::InvalidIndex};

    friend class Component;

protected:
    explicit CacheVariableDetails(std::string _name) :
        name{std::move(_name)} {
    }

    // A default constructor is required for user-facing impl. because
    // downstream classes might not be able to fully initialize an instance
    // at construction time (e.g. via member initialization).
    //
    // Usage of a partially-initialized CacheVariable is invalid. The way
    // that this is dealt with is by checking things at runtime:
    //
    // - This will default-initialize with a name of "", which is disallowed
    //   and should produce an appropriate warning (e.g. 'are you sure you
    //   called `Component::createCacheVariable`?)
    //
    // - The index is default-initialized, and copy constructed, with a
    //   SimTK::NaN, so downstream classes can't alias cache variables by
    //   copying this class around.
    CacheVariableDetails() = default;
};

/**
 * A cache variable containing a value of type T.
 *
 * - This is created via Component::addCacheVariable
 * - The Component:: base class contains methods for manipulating this (e.g. Component::getCacheVariableValue)
 *
 * @tparam T
 *   Type of data held in the cache variable
 */
template<class T>
class CacheVariable : public CacheVariableDetails {
private:
    friend class Component;

    explicit CacheVariable(std::string _name) : CacheVariableDetails{std::move(_name)} {
    }

public:
    // See default construction justification in `CacheVariableDetails`. In effect,
    // public default construction must be allowed so that downstream users can declare
    // a `CacheVariable<T>` in their class without having to fully initialize it upon
    // construction of that class.
    CacheVariable() = default;
};

Where Component is responsible for keeping its invariants correct. The most important one being that it falls back to a std::map lookup if the index isn't known.
SimTK::CacheEntryIndex Component::getCacheVariableIndex(const CacheVariableDetails& cv) const
{
    // cheap: index previously initialized, just return that
    if (cv.maybeUninitIndex != SimTK::InvalidIndex) {
        return cv.maybeUninitIndex;
    }

    // expensive: perform index lookup and initialize it

    if (cv.name.empty()) {
        std::stringstream msg;
        msg << "Component::getCacheVariableIndex(CacheVariable<T>&): cannot get cache variable index: the cache variable has no name: has it been initialized with Component::addCacheVariable? (class = " << getClassName() << ", concrete class name = " << getConcreteClassName() << ")";
        throw Exception{msg.str(), __FILE__, __LINE__};
    }

    // getCacheVariableIndex asserts whether the returned index is valid or not,
    // so this assignment will set the index to something valid, making subsequent
    // calls use the cheap path (above).
    cv.maybeUninitIndex = this->getCacheVariableIndex(cv.name);

    return cv.maybeUninitIndex;
}

The ugly-looking name check allows downstream classes to default-initialize a CacheVariable<T> without a name. Better C++ would just fully initialize it in each classes ctor/member initializer, but downstream classes typically rely on default initialization, so I'm making sure there's an obvious error message if they forget to call addCacheVariable at runtime.
Overall, this design means that, once cached, using a CacheVariable<T> should be equivalent to using a raw index, with one branch hit (the initial cache check) that should be branch-predicted fairly cheaply (it'll always be true once initialized)

But effectively it's an opaque handle that can only be received from and given to a Component at the moment: users of CacheVariable<T> can't manipulate its data or call any member methods.
The reason I went for this approach, rather than having a full-fat CacheVariable, is to keep things similar-looking to the old API. I also didn't want CacheVariable<T> to hold a reference to the Component it's created from (a prerequisite, if it wants to manipulate anything) because that could end up in all kinds of knots if (e.g.) the Component is moved around.
Also I'm currently using the template type (T) redundantly: it isn't doing much right now, apart from enabling template-type-deduction to work on some of the methods (e.g. getCacheVariableValue<T>). I put it there because the resulting type signature (API-wise) will be forward-compatible with optimizations such as optimizing out the dynamic_cast that  currently happens at runtime (a few % of perf in some simulations).

Thanks! Two questions:

Isn't the use of underscores in _name opposite from the rest of OpenSim? I thought they prefix members with _ instead of parameters (sorry if I'm mistaken)?
Why the separate CahceVariableDetail?

@tgeijten

you're right - I'll rename the members. This is my style, not OpenSim's, and I'll go fix that.

Because the underlying storage container in Component is std::map<string, CacheVariableDetail>. C++ recognizes different template-types as entirely different types, so I can't do something like std::map<string, CacheVariable<any?>>. There's also non-templated methods like isCacheVariableValid, which could take a CacheVariable<T>, but the T would be unused, so I'm having those methods take the type-erased version so that the implementation can sit in the implementation file rather than the header

I see. Do I understand correctly that you're storing those cache variables separately in a std::map only for legacy support? Is the 'correct' way to simply keep CacheVariable<T> members in your class? In which case, wouldn't you then have two copies of each CacheVariable and three copies of each std::string holding the cache variable name (including the one in the std::map)?

Yeah, there's currently redundant copies of the key, where the CacheVariable owns one and the underlying map owns another.
I tried reference-counted strings (e.g. a string in a shared_ptr) but found that it was a fair bit uglier, code-wise, and guessed it's unlikely to have any perf impact. On the major platforms (IIRC), there's short-string optimization on strings <= 15 chars (on 64-bit systems), and cache vars are typically short (e.g. arm_velocity), so there's usually no allocation and, so the memory hit for redundancy is ~32 bytes (string + index) per CacheVariable.

Ah, I see what you mean though, because the std::map could just hold the SimTK index, rather than redundantly holding it as well as it being held in the std::map's entry pair - I can go and remove that.

Do you also have to keep the name inside the CacheVariable?
Performance-wise, I'm more worried about the extra memory required for each Component that uses CacheVariables and not the allocation, which happens only during initialization.

@tgeijten I think so, because pre-index-caching, the CacheVariable needs to be able to lookup the cache variable in the base Component (because it's the Component that, later, allocates the SimTK cache variable, yielding the index). The underlying storage model (map) requires that the lookup uses a string key, so the caller needs to (at least) be able to construct that key at runtime.
I experimented with the idea of requiring that CacheVariables have a compile-time C-string, because almost all CacheVariables have string names that are known at compile-time (effectively, string literals), but there are some instances of using computed names, which would torpedo that idea. I also experimented with using (e.g.) a std::vector as Component's backing storage. This removes the need for a string key--just hold an index instead--but this breaks the requirement that this change is API-compatible with the old model of using string lookups at runtime (they would perform much more poorly).

w.r.t. how many strings are stored, I've completely mis-remembered what is actually held in the map. There's a third class used for storage, StoredCacheVariable:
/**
 * A cache variable, as stored internally by Component (internal).
 */
struct StoredCacheVariable {
    SimTK::ClonePtr<SimTK::AbstractValue> value;
    SimTK::Stage dependsOnStage;
    SimTK::ResetOnCopy<SimTK::CacheEntryIndex> maybeUninitIndex{SimTK::InvalidIndex};

    StoredCacheVariable(SimTK::AbstractValue* _value,
                        SimTK::Stage _dependsOnStage) :
            value{_value},
            dependsOnStage{_dependsOnStage} {
    }

    SimTK::CacheEntryIndex index() const {
        if (this->maybeUninitIndex != SimTK::InvalidIndex) {
            return this->maybeUninitIndex;
        } else {
            const char* msg = "StoredCacheVariable::get: failed because this->index == SimTK::InvalidIndex: this can happen if Component::extendRealizeTopology has not been called";
            throw Exception{msg, __FILE__, __LINE__};
        }
    }
};

// Map names of cache entries of the Component to their individual
// cache information.
mutable std::unordered_map<std::string, StoredCacheVariable> _namedCacheVariables;

So the string is only duplicated once (one for the map, one for the CacheVariable), and the underlying storage is just holding the relevant value prototype (value), potentially-initialized SimTK index, and the stage. I considered compacting this into a union that switches between NotInitializedStoredCacheVariable and InitializedStoredCacheVariable that std::moves the prototype into SimTK, rather than copying it, but I figured that'd be quite complicated, and would make realizeTopology etc. potentially explode if called twice

I see. So the CacheVariableDetails are not stored in a container? Is it then still needed as a separate base class?

@tgeijten it's taken as a parameter to non-templatized methods:
SimTK::CacheEntryIndex getCacheVariableIndex(const CacheVariableDetails& cv) const;
bool isCacheVariableValid(const SimTK::State& state, const CacheVariableDetails& k) const;
void markCacheVariableValid(const SimTK::State& state, const CacheVariableDetails& name) const;
void markCacheVariableInvalid(const SimTK::State& state, const CacheVariableDetails& name) const;

We could just switch these methods to take a CacheVariable<T>, which would be an easy refactor to do, so I guess the tradeoff is:

Make CacheVariable<T> simpler, by removing a base class, but have more code in a header

vs.

I would even go as far as remove these functions from the Component API entirely. Instead, add member functions to CacheVariable that take a const SimTk::State&. This way there's a clear distinction between the (slow) legacy API and the new fast one.

Doing that properly would require that the user handles cache variable initialization correctly. As in, they perform this step, which is "automatically" done by Component in extendRealizeTopology:
        for (const std::string& k : keys) {
            StoredCacheVariable& cv = this->_namedCacheVariables.at(k);
            cv.maybeUninitIndex = subSys.allocateLazyCacheEntry(s, cv.dependsOnStage, cv.value->clone());
        }

Tbh, it's a fair idea, in that CacheVariable could just be a safe wrapper that ensures that the caller can't get the index until it's been properly initialized in SimTK. My only concern is whether upstream + other maintainers are going to be cool with the idea of doing something "low level" like this in the implementations themselves.
Ultimately, all of this work is to keep things API-compatible while yielding similar perf benefits to just using SimTK directly. The Component implementation ultimately jumps through all of these hoops to call subSys.allocateLazyCacheEntry(s, cv.dependsOnStage, cv.value->clone()); at the right time.

Well, either way is fine with me. I do agree not having a CacheVariableDetails base class is better.

I mean, perf-wise, some of the simulations in OpenSim do seem to spend a fair bit of time doing the hoop-jumping related to holding things in vectors, maps, etc. and downcasting them at runtime.
Being able to just directly allocate values in a class and use them directly without lookups/downcasting would be lovely. It's something I've been wanting to look into (e.g. by changing the various PROPERTY macros to directly initialize values on components, rather than allocating them via AbstractValues and Vectors etc.). However, that's going to be tricky for me to do right because various parts of OpenSim rely heavily on the ability to reflect over properties, enumerate properties, get object names, etc. etc. at runtime. We've effectively reimplemented Java/Python's memory model in C++ and are now experiencing situations in which our C++ runs at the speed of Java :D.
For some context, my primary goal--at least for now--is to try and improve OpenSim without changing things too much, because I'm assuming that the biomechanists, researchers, original devs, etc. know a lot more than me about OpenSim's design (it's gradually "matured" over years), so I'm guessing that it would be foolish of me to heavily change any APIs - at least while I'm the "new guy". With that in mind, I'm implementing things that might be sub-optimal, but are close enough to the previous design to not shake the boat.
For personal projects, or anything greenfield, I'm slamming the SIMD/Rust/struct packing to the max though :D

On another note, I've managed to port my perf suite to work on Mac OSX now (and it should work on Windows, which I'll check tomorrow), so I've now got some basic perf comparisons for a Macbook Air:

Test Name
lhs [secs]
σ [secs]
rhs [secs]
σ [secs]
Speedup

Gait2354
0.35
0.04
0.37
0.04
0.96

ToyDropLanding
13.55
0.08
12.64
0.05
1.07

ToyDropLanding_nomuscles
0.53
0.00
0.54
0.00
0.99

passive_dynamic
4.70
0.00
4.68
0.00
1.00

passive_dynamic_noanalysis
3.00
0.00
3.00
0.00
1.00

Arm26
0.33
0.00
0.30
0.00
1.10

RajagopalModel
13.56
0.02
13.71
0.01
0.99

Tbh, kind of underwhelming. Expected, though: the perf in models like ToyDropLanding is peppered all over the place (due to the memory model) so most of the perf changes are only going to be 5% here and 10% there for those. I'm hoping that the muscle wrapping stuff has a solution, because those sims spend ~50 % of their time in one function (applyWrap).
Using a CacheVariable<T> to elilde the runtime dynamic_casts should be another small boost, and can be done in an API-compatible way (because it's strongly-typed).

@aseth1 pointed out one extra issue, which is that there are no guards against accidently using the same cache variable name in two different cache variables. For example, this is currently completely legal:
CacheVariable<double> cv1 = addCacheVariable("foo", 10.0, Stage::Time);
CacheVariable<short> cv2 = addCacheVariable("foo", 5, Stage::Time);

When the second line executes, the implementation will just overwrite the foo entry in the map. A runtime issue will not occur until later, when cv1 is used at runtime:
double d = getCacheVariableValue(cv1, state);  // or similar

Before the SimTK index is cached inside the CacheVaraible, this method will perform a string lookup for foo, retrieve the resulting index for that, and use the index to retrieve the underlying AbstractValue. A runtime dynamic_cast will then happen (somewhere) into a SimTK::Value<double>, which will fail and (hopefully) show some useful error message.
That sequence of events is non-obvious from a developer's PoV, because it will happen later and show an  unrelated cast message. The solution to this is to prevent addCacheVariable from allowing the variable to be added if a variable with the same name has already been added.
This is also a bug in the previous API, which would result in a similar error. However, it should be fixed in this update because one potential optimization (later) would be to elilde the dynamic_cast and instead use a reinterpret_cast at runtime, which is unsafer (there's no runtime type checks), but faster. With the currently unguarded behavior, this could result in very nasty bugs at runtime.

This version checks for duplicate entries, but fails in exampleMuscle with:
adam@akmain:~/Desktop/opensim-core$ ./opensim-core-build/exampleMuscle
[warning] Couldn't find file 'ground.vtp'.
[warning] Couldn't find file 'anchor1.vtp'.
[warning] Couldn't find file 'anchor2.vtp'.
[error] Mesh block.vtp not connected to model...ignoring
[warning] Couldn't find file 'block.vtp'.
Component::addCacheVariable(...): cannot create a cache variable with the name 'target_activation_deriv' because another cache variable with that name already exists (class = Component, concrete class name = FatigableMuscle)

Which implies that the tested muscle, FatigableMuscle, double-adds the cache variable. The implementation doesn't seem to obviously do this, but it's actually a legit because this code is in FagitableMuscle.cpp:
// Define new states and their derivatives in the underlying system
void FatigableMuscle::extendAddToSystem(SimTK::MultibodySystem& system) const
{
    // Allow Millard2012EquilibriumMuscle to add its states, before extending
    Super::extendAddToSystem(system);

    // Now add the states necessary to implement the fatigable behavior
    addStateVariable("target_activation");
    addStateVariable("active_motor_units");
    addStateVariable("fatigued_motor_units");
    // and their corresponding derivatives
    addCacheVariable("target_activation_deriv", 0.0, SimTK::Stage::Dynamics);
    addCacheVariable("active_motor_units_deriv", 0.0, SimTK::Stage::Dynamics);
    addCacheVariable("fatigued_motor_units_deriv", 0.0, SimTK::Stage::Dynamics);
}

But this code is in Component.cpp:
    const AddedStateVariable* asv =
        dynamic_cast<const Component::AddedStateVariable *>(stateVariable);
    // Now automatically add a cache variable to hold the derivative
    // to enable a similar interface for setting and getting the derivatives
    // based on the creator specified state name
    if(asv){
        addCacheVariable(stateVariableName+"_deriv", 0.0, Stage::Dynamics);
    }

So there's this cute side-effect in addStateVariable where it automatically adds a _deriv state variable for each added state variable at runtime to the cache variables, which now causes the duplication check to throw because the test explicitly tries to add the cache variable also. Two solutions to this:

Remove adding the _deriv cache variable in FagitableMuscle.cpp:90, because addStateVariable adds it anyway
Allow cache variables ending with _deriv to be double-inserted (do not duplicate-check them)

The first option fixes the test and keeps the strict semantics, but might break external users that redundantly add the _deriv cache variable. The second option keeps backwards compatability at the risk of quietly swallowing a duplicate entry if it ends with _deriv (there will still be a runtime bug). The second option also potentially removes the opportunity to perform reinterpret_casts later, unless we also introduce some horrible-looking architecture to handle the edge case that variables specifically ending in _deriv should always be dynamic_casted because an overwrite may have changed the underlying datatype.
@aseth1 should I just go fix the test or do you think that people are going to be manually adding _deriv variables a lot in the wild?

Spoke about this in person with Ajay. The solution is to remove the _deriv cache variables from the test case because they're technically invalid now.

This has, after an insane amount of faffing around, finally been merged into master.


Conflict with Controlnet and Reactor in img2img - ok in mov2mov tab

If the mov2mov extension is enabled, I get the following error when trying to use the Reactor extension with img2img:
2023-11-09 20:36:00,279 - ControlNet - WARNING - No ControlNetUnit detected in args. It is very likely that you are having an extension conflict.Here are args received by ControlNet: (). 2023-11-09 20:36:00,279 - ControlNet - WARNING - No ControlNetUnit detected in args. It is very likely that you are having an extension conflict.Here are args received by ControlNet: (). *** Error running process: E:\StableDiffusion\Automatic1111\stable-diffusion-webui-1.6\extensions\sd-webui-reactor\scripts\reactor_faceswap.py     Traceback (most recent call last):       File "E:\StableDiffusion\Automatic1111\stable-diffusion-webui-1.6\modules\scripts.py", line 619, in process         script.process(p, *script_args)     TypeError: FaceSwapScript.process() missing 21 required positional arguments: 'img', 'enable', 'source_faces_index', 'faces_index', 'model', 'face_restorer_name', 'face_restorer_visibility', 'restore_first', 'upscaler_name', 'upscaler_scale', 'upscaler_visibility', 'swap_in_source', 'swap_in_generated', 'console_logging_level', 'gender_source', 'gender_target', 'save_original', 'codeformer_weight', 'source_hash_check', 'target_hash_check', and 'device'
Disabling the mov2mov extension resolves the issue in the img2img tab.
There are no errors however when using Reactor in the mov2mov extension tab.

I found this problem too. Any news?

the last time I updated, I didn't get this error, it should be related to the recent update of WebUI or Controlnet. I'll try to update it sometime.


Adds Publish action to RabbitMQ

This commit adds a publish action to the RabbitMQ pack, allowing a crafted message to be published to a RabbitMQ server.

@manasdk switched to Pika. Let me know how this looks. I can squash the commits to something cleaner before the final merge.

Looks good. Thanks for switching to Pika.
Travis is complaiing about line length -
/home/travis/build/StackStorm/st2contrib/packs/rabbitmq/actions/publish_message.py:6:101: E501 line too long (129 > 100 characters)
Will merge after that gets fixed.

@manasdk looks like everything is checking out. Let me know if there are any other changes to make.

Done. Merged!

Nice :+1:


[BUG] 0.12.2 Missing opportunity to delete Users

Describe the bug
If i want to delete a admin-user or allowed-user in section "Server--> Users" there is no more possibility to do that.
To Reproduce
Steps to reproduce the behavior:

Go to section "Server--> Users"
Add a user by user id
Try to delete it afterwards
See missing opportunity

Screenshots

Version

LNbits version: 0.12.2

fixed


Deprecate JSON Database

The JSON database is inefficient and problematic. Removing it will allow for great improvements in areas such as: User Iteration for Reminders.
JSON entities will be deprecated for 3 days, after which they will be removed.
(This is to provide an example of how features should be deprecated. While JSON could just be removed, its better to deprecate it. A relatively short period is used because this removal is rather insignificant. More important features should have longer deprecation windows.)

Issue is NOT closed until removal occurs

I got sick and it didn't get done yet....
I'll do it soon (tm)

Closed with Pull #13


NullReferenceException in AndroidDriver.ctor

I use Appium server running on Mac machine and AppiumDriver with c# bindings to execute tests from Windows machine. The issue is that when I create an instance of AndroidDriver its ctor rises a NullReferenceException. When I create an instance of RemoteWebDriver, it works smoothly. This is my code:
var caps = new DesiredCapabilities();
caps.SetCapability(MobileCapabilityType.DeviceName, "a0d86205");
caps.SetCapability("udid", "a0d86205");
caps.SetCapability(MobileCapabilityType.App, "https://valid/path/to/my.apk");
caps.SetCapability(MobileCapabilityType.PlatformName, MobilePlatform.Android);

var drv = new AndroidDriver(ServerUri, caps) // rises exception
// var drv = new RemoteWebDriver(ServerUri, caps) // works smoothly

Exception:
System.NullReferenceException : Object reference not set to an instance of an object.
Result StackTrace:
at OpenQA.Selenium.Appium.AppiumDriver._AddAppiumCommands()
at OpenQA.Selenium.Appium.AppiumDriver..ctor(Uri remoteAddress, ICapabilities desiredCapabilities)
at OpenQA.Selenium.Appium.Android.AndroidDriver..ctor(Uri remoteAddress, DesiredCapabilities desiredCapabilities)

Most recent server responce
info: [debug] Responding to client with success: {"status":0,"value":{"platform":"LINUX","browserName":"Android","platformVersion":"5.0.1","webStorageEnabled":false,"takesScreenshot":true,"javascriptEnabled":true,"databaseEnabled":false,"networkConnectionEnabled":true,"locationContextEnabled":false,"warnings":{},"desired":{"udid":"a0d86205","deviceName":"a0d86205","platformName":"Android","app":"https://valid/path/to/my.apk"},"udid":"a0d86205","deviceName":"a0d86205","platformName":"Android","app":"https://valid/path/to/my.apk"},"sessionId":"9536382e-a320-4d8b-b8cd-3afdd4c0bd68"}
info: <-- GET /wd/hub/session/9536382e-a320-4d8b-b8cd-3afdd4c0bd68 200 0.702 ms - 641 {"status":0,"value":{"platform":"LINUX","browserName":"Android","platformVersion":"5.0.1","webStorageEnabled":false,"takesScreenshot":true,"javascriptEnabled":true,"databaseEnabled":false,"networkConnectionEnabled":true,"locationContextEnabled":false,"warnings":{},"desired":{"udid":"a0d86205","deviceName":"a0d86205","platformName":"Android","app":"https://valid/path/to/my.apk"},"udid":"a0d86205","deviceName":"a0d86205","platformName":"Android","app":"https://valid/path/to/my.apk"},"sessionId":"9536382e-a320-4d8b-b8cd-3afdd4c0bd68"}

Where am I wrong? Why the constructor of AndroidDriver rises this execption and doesn't specify the reason?

try using the new driver i'm about to publish.  I just tried it with your exact settings above and I did not get an error  on construction.


Chatbot2.0

Issue #, if available: Addressing user experience issues
Description of changes: This PR contains support for general chat along with RAG conversations. Also added support for Claude 3 models.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Tested in the workshop studio and ready to merge.


Unsubbed/Unfavorited On Workshop Page Mod Still In Game

I can't figure out where I need to go to delete the mod. All the help I find online is not helpful for this. Everyone says to go to Steam/Steamapps/Workshop/Content or something similar and there is nothing. I search for civ6 mods or a workshop folder on my entire computer and can't find it anywhere.
CQUI shows up installed in game but not subscribed to. I've verified game files and reinstalled twice and it still shows up.
I have reached a breaking point, please help...

The two paths that come to my mind are:
...\steam\steamapps\common\Sid Meier's Civilization VI
and
\Documents\My Games\Sid Meier's Civilization VI

Have you tried disabling simply disabling it from the list of mods in the additional content menu in game?
Example image of additional content menu

If you are trying to continue a save game that you had previously started with that mod I THINK that mod code is actually bundled up with the save. I can't verify that but I have had problems in the past that appeared to be the mod being a part of a save game I was continuing even though it had been deleted. Maybe someone else can confirm this...

No, it was not a saved game.
The game still has fps issues if i disable CQUI, I have to fully unsub and restart the game in order for it to go away the first time.
I checked every path for steam and civ6 and none of them have files in any folders named content or mods.

I'm retarded. Forgot I moved my steam folder to my game SSD from my HDD.
Was looking in the wrong directory.


Generate content id when absent

This is a rough proposal for working around the missing content_id values for hrmc_manual and hmrc_manual_section formats.
Full story here https://trello.com/c/pULByuHs/393-fix-invalid-content-store-data
The uuid is generated consistently from the base_path of the manual/section, it's a valid v5 uuid as per RFC4122. It's also discernible from a typical SecureRandom.uuid as these are v4.
/cc @jennyd Can we talk about this when you have a moment.

Recap from irl conversation, @rboulton and I approve of this. We will ask the HMRC devs to use the content-ids generated with this method for existing content. :+1:

I don't think this would handle slugs being changed - though if I recall correctly, hmrc-manuals-api doesn't support that anyway?

This seems like a reasonable approach to unblock the import :+1:
The API doesn't currently support slugs being changed. It is possible that manual slugs might need to change in future, which this approach wouldn't support. If a manual slug changes then the base path for all its sections will also change.
We could tell HMRC that we're taking this approach now to unblock the import, and that it will prevent them from changing manual slugs in future (after they've been published) unless they are first able to start sending content ids.
We'll also need to add the content ids into the gone representations of manuals and sections.

Thanks all for comments and review, moved this PR to https://github.com/alphagov/hmrc-manuals-api/pull/122


Uncaught TypeError: Cannot read property 'prototype' of undefined

Please i'm facing a big problem with this issue and i need help
the mapToClass function is giving me an issue.
the program faill at this part of the code
mapToClass: function (constructor, structure) {             var instanceTemplate = Object.create(constructor.prototype);
i use dexie 1.5 in my IONIC 2 project
here is the minimum project where i register this error https://github.com/stchoukeu/IONIC2-RC4-with-DEXIE-Database

Might be that your build system doesn't handle circular dependencies (database.ts <--> personne.ts). Try pasting in all code from personne.ts into database.ts and see if it helps.


exec dispatcher doesnt seem to apply dynamic rules.

Bug or Regression?
Bug
System Info and Version
 System/Version info
Hyprland, built from branch  at commit ea2501d4556f84d3de86a4ae2f4b22a474555b9f  ().
Date: 2024-06-10
Tag: , commits:

flags: (if any)

System Information:
System name: Linux
Node name: ssd
Release: 6.6.32
Version: #1-NixOS SMP PREEMPT_DYNAMIC Sat May 25 14:22:56 UTC 2024

GPU information:
01:00.0 VGA compatible controller [0300]: NVIDIA Corporation GA106 [GeForce RTX 3060 Lite Hash Rate] [10de:2504] (rev a1) (prog-if 00 [VGA controller])

os-release: ANSI_COLOR="1;34"
BUG_REPORT_URL="https://github.com/NixOS/nixpkgs/issues"
BUILD_ID="24.05.20240530.d24e7fd"
DOCUMENTATION_URL="https://nixos.org/learn.html"
HOME_URL="https://nixos.org/"
ID=nixos
IMAGE_ID=""
IMAGE_VERSION=""
LOGO="nix-snowflake"
NAME=NixOS
PRETTY_NAME="NixOS 24.05 (Uakari)"
SUPPORT_END="2024-12-31"
SUPPORT_URL="https://nixos.org/community.html"
VERSION="24.05 (Uakari)"
VERSION_CODENAME=uakari
VERSION_ID="24.05"

plugins:

======Config-Start======

Config File: /home/usr/.config/hypr/hyprland.conf: Read Succeeded
exec-once = /nix/store/bnipzdfv788l7yvgn7k7s7gp76sd1zpz-dbus-1.14.10/bin/dbus-update-activation-environment --systemd DISPLAY HYPRLAND_INSTANCE_SIGNATURE WAYLAND_DISPLAY XDG_CURRENT_DESKTOP && systemctl --user stop hyprland-session.target && systemctl --user start hyprland-session.target
$mod=SUPER
animations {
  bezier=myBezier, 0.05, 0.9, 0.1, 1.05
  animation=windows, 1, 5, myBezier
  animation=windowsOut, 1, 5, default, popin 80%
  animation=border, 1, 5, default
  animation=borderangle, 1, 8, default
  animation=fade, 1, 7, default
  animation=workspaces, 1, 6, default
}

decoration {
  blur {
    enabled=true
    passes=1
    size=5
    vibrancy=0.169600
  }
  rounding=5
}

input {
  kb_layout=de
}

bind=, code:122, exec, pamixer -d 5
bind=, code:123, exec, pamixer -i 5
bind=, code:173, exec, playerctl previous
bind=, code:171, exec, playerctl next
bind=, code:172, exec, playerctl play-pause
bind=$mod, F, exec, firefox
bind=, Print, exec, grimblast copy area
bind=$mod, M, exit
bind=$mod, T, exec, alacritty
bind=$mod, Q, killactive
bind=ALT, space, exec, wofi --show=drun
bind=CTRL_SHIFT, S, exec, grim -g "$(slurp)"  - | tee >(wl-copy) >"/home/usr/$(date +'%y%m%d_%Hh%Mm%Ss')_grim.png"
bind=SUPER_SHIFT, C, exec, cliphist list | wofi --show=dmenu | cliphist decode | wl-copy
bind=$mod, 1, workspace, 1
bind=$mod SHIFT, 1, movetoworkspace, 1
bind=$mod, 2, workspace, 2
bind=$mod SHIFT, 2, movetoworkspace, 2
bind=$mod, 3, workspace, 3
bind=$mod SHIFT, 3, movetoworkspace, 3
bind=$mod, 4, workspace, 4
bind=$mod SHIFT, 4, movetoworkspace, 4
bind=$mod, 5, workspace, 5
bind=$mod SHIFT, 5, movetoworkspace, 5
bind=$mod, 6, workspace, 6
bind=$mod SHIFT, 6, movetoworkspace, 6
bind=$mod, 7, workspace, 7
bind=$mod SHIFT, 7, movetoworkspace, 7
bind=$mod, 8, workspace, 8
bind=$mod SHIFT, 8, movetoworkspace, 8
bind=$mod, 9, workspace, 9
bind=$mod SHIFT, 9, movetoworkspace, 9
bind=$mod, 0, workspace, 10
bind=$mod SHIFT, 0, movetoworkspace, 10
bind=, F1, togglespecialworkspace, E
bindm=$mod, mouse:274, movewindow
bindm=$mod, mouse:272, resizewindow
env=XDG_CACHE_DIR,/home/usr/.cache/
env=XDG_CONFIG_HOME,/home/usr/.config/
env=ELECTRON_OZONE_PLATFORM_HINT,auto
env=NIXOS_OZONE_WL,1
exec-once=hyprpaper
exec-once=wl-paste --type text --watch cliphist store
exec-once=wl-paste --type image --watch cliphist store
exec-once=ags
monitor=Unknown-1, disable

======Config-End========

( Im building from the latest nix flake in this repo )

Description
Long story short using dynamic rules with the exec dispatcher, like this:
hyprctl dispatch "exec [rounding 10000] alacritty"

doesn't work, however static ones, like this one:
hyprctl dispatch "exec [float] alacritty"

Works perfectly fine. Worth noting is that hyprctl returns ok in any case (even when the rules are misspelled)
Is this intended? If so I was unable to find any mention of it in the wiki.
How to reproduce
Run
hyprctl dispatch "exec [rounding 10000] alacritty"

Crash reports, logs, images, videos
https://github.com/hyprwm/Hyprland/assets/81042605/8586e731-cf66-4397-a0fb-39aa06a137dd

that's expected behavior. For dynamic rules, use windowrules.

Why is there seemingly no mention of it in the wiki then?

MRs welcome.

Removing the ending part from the config manager file will do it, likely without causing additional issues iirc.
I once thought about properly passing them onto the SWindowAdditionalConfigData but ended up never doing it.
for (auto& er : execRequestedRules) {
    if (std::ranges::any_of(PIDs, [&](const auto& pid) { return pid == er.iPid; })) {
        returns.push_back({er.szRule, "execRule"});
        anyExecFound = true;
    }
}

if (anyExecFound && !shadowExec) // remove exec rules to unclog searches in the future, why have the garbage here.
    execRequestedRules.erase(std::remove_if(execRequestedRules.begin(), execRequestedRules.end(),
                                            [&](const SExecRequestedRule& other) { return std::ranges::any_of(PIDs, [&](const auto& pid) { return pid == other.iPid; }); }));

it will cause a memory leak :)

it will cause a memory leak :)

yes, but it's probably fine for a daily session with a few exec rules being used.
my initial idea was to erase them only once the process was gone but had no idea how to do (properly) that back then

add a timer every 60s to check if /proc/$pid exists for all rules and remove if it doesnt
and then let the bsd guys fix it for bsd

and then let the bsd guys fix it for bsd

yeah that was my main concern


Add simple way to include server's current time in query payload

Certain situations require knowledge of the server's current time at the time a query payload is sent to the client. Specifically, timers that need to be synchronized across different clients whose local times will be different would need this knowledge.
Per GC support's recommendation, I'm going to implement a workaround by mutating a single Time node and retrieving its updatedAt field value. A simpler solution would be desirable, especially one that would allow the currentServerTime to be appended to a query payload.

Thanks for putting this request out there 🙌

The recently released resolver functions should solve this use case :-)


handle downloading files without curl

no need for curl anymore
no need for --allow-run
would need --allow-write

done


Stop processing client connections when the client is disconnected

Without this change, a client going away without issuing a QUIT will cause the RedisClient to spin forever, throwing EOFExceptions that are turned into ParseErrorException, but it never stops.
The main change is allowing EOFException to bubble up to the RedisClient, where we'll now close the streams and unset the running flag, so the thread can exit.
The Collection<RedisClient> was added so we can test this easily, but not exposed to the public API.

Hi, thank you for your PR. I hesitated to merge it because it does not contain a test that reproduces the problem (it's possible to write such test, however this involves writing complex mocks). Then #135 came with a one-line workaround, and I think it's a better for this case


i2c.write hangs with a NULL buffer

micropython has a function i2c.scan, which hangs V2 when an external i2c device is connected.
As far as I understand the micropython source, this is implemented by writing a NULL buffer to each address.
Have I got it right @dpgeorge ? I'm puzzled because my sample doesn't detect devices on a V1, whereas i2c.scan does, so I must be doing something wrong!
V2 i2c.write hangs when it gets to an attached i2c device - maybe stuck in waitForStop.
With the sample below, Press A to write to each address; B to read from each address.
#include "MicroBit.h"

#ifdef CODAL_CONFIG_H
#define MICROBIT_CODAL 1
#else
#define MICROBIT_CODAL 0
#endif

MicroBit uBit;

void i2c_scan_write()
{
    for (int addr = 0x08; addr < 0x78; ++addr)
    {
        int ret = uBit.i2c.write( addr << 1, NULL, 0, false);

        uBit.serial.printf( "write addr %x = %d\n", addr << 1, ret);
    }
}

void i2c_scan_read()
{
#if MICROBIT_CODAL
    uint8_t data[100];
#else
    char data[100];
#endif

    for ( int addr = 0x08; addr < 0x78; addr++)
    {
        memset( data, 255, 100);
        int ret = uBit.i2c.read( addr << 1, data, 1, false);
        uBit.serial.printf( "read addr %x = %d data[0] = %d\n", addr << 1, ret, (int) data[0]);
    }
}

void onButtonA(MicroBitEvent e)
{
  i2c_scan_write();
}

void onButtonB(MicroBitEvent e)
{
  i2c_scan_read();
}

void togglePixel( int x, int y)
{
    uBit.display.image.setPixelValue( x, y, uBit.display.image.getPixelValue( x, y) ? 0 : 255);
}

void forever()
{
    while (true)
    {
      togglePixel( 0, 0);
      uBit.sleep(1000);
    }
}

int main()
{
    // Initialise the micro:bit runtime.
    uBit.init();

    create_fiber( forever);

    uBit.messageBus.listen(MICROBIT_ID_BUTTON_A, MICROBIT_BUTTON_EVT_CLICK, onButtonA);
    uBit.messageBus.listen(MICROBIT_ID_BUTTON_B, MICROBIT_BUTTON_EVT_CLICK, onButtonB);

    // If main exits, there may still be other fibers running or registered event handlers etc.
    // Simply release this fiber, which will mean we enter the scheduler. Worse case, we then
    // sit in the idle task forever, in a power efficient sleep.
    release_fiber();
}

As far as I understand the micropython source, this is implemented by writing a NULL buffer to each address.

Yes, that's the best way to scan for devices, to do an empty write; ie just send the address with a write selected (lowest bit clear?) and then a stop condition.

Thanks @dpgeorge @martinwork
You were quite right @martinwork. The code was stuck inside waitForStop(). The NRF52 hardware doesn't really like zero length write operations... it doesn't seem to have a mechanism to relay success - only error cases...
Anyhow, I've applied a patch here that introduces a special case workaround. This seems to work ok on the internal I2C bus on a v2 micro:bit using similar code to @martinwork above:
https://github.com/lancaster-university/codal-nrf52/commit/1a8624801d45301e0f5dfc2c1a17a5efca8082ab
Timeout is currently set to around 1ms, which seems about right (that would be 10 bytes at 100kHz bus speed). Happy to adjust if anyone has a better suggestion for the timeout period.

Trying this in the current CODAL, with the external I2C bus without anything connected to it and I get all odd 7-bit addresses as "available":
#include "MicroBit.h"

MicroBit uBit;

int main() {
    uBit.init();

    uBit.serial.printf("Available addresses: ");
    // https://github.com/microbit-foundation/micropython-microbit-v2/blob/a76e1413bcd66f128a31d98756fc3d1f336d1580/src/codal_port/microbit_i2c.c#L69
    // 7-bit addresses 0b0000xxx and 0b1111xxx are reserved
    for (int addr = 0x08; addr < 0x78; ++addr) {
        int ret = uBit.i2c.write(addr << 1, NULL, 0, false);
        if (ret == MICROBIT_OK) {
            uBit.serial.printf("%d, ", addr);
        }
    }
    uBit.serial.printf("\n\n");
    for (;;);
}

Available addresses: 9, 11, 13, 15, 17, 19, 21, 23, 25, 27, 29, 31, 33, 35, 37, 39, 41, 43, 45, 47, 49, 51, 53, 55, 57, 59, 61, 63, 65, 67, 69, 71, 73, 75, 77, 79, 81, 83, 85, 87, 89, 91, 93, 95, 97, 99, 101, 103, 105, 107, 109, 111, 113, 115, 117, 119,

Changing that to the internal _i2c instance and I get odd numbers until address 29, and all event numbers immediately after that.
Available addresses: 9, 11, 13, 15, 17, 19, 21, 23, 25, 27, 29, 30, 32, 34, 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62, 64, 66, 68, 70, 72, 74, 76, 78, 80, 82, 84, 86, 88, 90, 92, 94, 96, 98, 100, 102, 104, 106, 108, 110, 112, 113, 114, 116, 118,

This is with an LSM with addresses 0x19 (25) and 0x1E (30). So taking in consideration the count starts at 0x08 it looks like it's basically just failing at every second i2c write.
If I switch back to the external i2c, but change the starting point to 0x09, then we get all even numbers:
Available addresses: 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30, 32, 34, 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62, 64, 66, 68, 70, 72, 74, 76, 78, 80, 82, 84, 86, 88, 90, 92, 94, 96, 98, 100, 102, 104, 106, 108, 110, 112, 114, 116, 118,

Running the I2c scan in MicroPython and we get a different result, where the "available" addresses change for each i2c.scan() call:

https://github.com/microbit-foundation/micropython-microbit-v2/issues/55#issuecomment-769941830
https://github.com/microbit-foundation/micropython-microbit-v2/blob/a76e1413bcd66f128a31d98756fc3d1f336d1580/src/codal_port/microbit_i2c.c#L69

Trying this in the current CODAL, with the external I2C bus without anything connected to it and I get all odd 7-bit addresses as "available":
#include "MicroBit.h"

MicroBit uBit;

int main() {
    uBit.init();

    uBit.serial.printf("Available addresses: ");
    // https://github.com/microbit-foundation/micropython-microbit-v2/blob/a76e1413bcd66f128a31d98756fc3d1f336d1580/src/codal_port/microbit_i2c.c#L69
    // 7-bit addresses 0b0000xxx and 0b1111xxx are reserved
    for (int addr = 0x08; addr < 0x78; ++addr) {
        int ret = uBit.i2c.write(addr << 1, NULL, 0, false);
        if (ret == MICROBIT_OK) {
            uBit.serial.printf("%d, ", addr);
        }
    }
    uBit.serial.printf("\n\n");
    for (;;);
}

Available addresses: 9, 11, 13, 15, 17, 19, 21, 23, 25, 27, 29, 31, 33, 35, 37, 39, 41, 43, 45, 47, 49, 51, 53, 55, 57, 59, 61, 63, 65, 67, 69, 71, 73, 75, 77, 79, 81, 83, 85, 87, 89, 91, 93, 95, 97, 99, 101, 103, 105, 107, 109, 111, 113, 115, 117, 119,

Changing that to the internal _i2c instance and I get odd numbers until address 29, and all event numbers immediately after that.
Available addresses: 9, 11, 13, 15, 17, 19, 21, 23, 25, 27, 29, 30, 32, 34, 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62, 64, 66, 68, 70, 72, 74, 76, 78, 80, 82, 84, 86, 88, 90, 92, 94, 96, 98, 100, 102, 104, 106, 108, 110, 112, 113, 114, 116, 118,

This is with an LSM with addresses 0x19 (25) and 0x1E (30). So taking in consideration the count starts at 0x08 it looks like it's basically just failing at every second i2c write.
If I switch back to the external i2c, but change the starting point to 0x09, then we get all even numbers:
Available addresses: 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30, 32, 34, 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62, 64, 66, 68, 70, 72, 74, 76, 78, 80, 82, 84, 86, 88, 90, 92, 94, 96, 98, 100, 102, 104, 106, 108, 110, 112, 114, 116, 118,

Running the I2c scan in MicroPython and we get a different result, where the "available" addresses change for each i2c.scan() call:

https://github.com/microbit-foundation/micropython-microbit-v2/issues/55#issuecomment-769941830
https://github.com/microbit-foundation/micropython-microbit-v2/blob/a76e1413bcd66f128a31d98756fc3d1f336d1580/src/codal_port/microbit_i2c.c#L69

I can confirm the results that @microbit-carlos gets just above (using pur C++ code, a list of many addresses are available, both on uBit.i2c and uBit._i2c buses; using MicroPython, the available addresses change).
Adding a delay after the I2C write seems to make the problem go away.  The delay needs to be about 30us for the external bus, and 10us for the internal bus.  With such delays the available addresses are correct (this is for C++ as was as MicroPython code).  Any less of a delay and you start to see wrong addresses being reported as available, ie the I2C write succeeds when it should fail.
For completeness this is my test code:
#include "MicroBit.h"

MicroBit uBit;

int main() {
    uBit.init();

    for (int count = 0; count < 10; ++count) {
        uBit.serial.printf("Available addresses: ");
        // 7-bit addresses 0b0000xxx and 0b1111xxx are reserved
        for (int addr = 0x08; addr < 0x78; ++addr) {
            int ret = uBit.i2c.write(addr << 1, NULL, 0, false);
            system_timer_wait_us(30); // delay to workaround issue with above call
            if (ret == MICROBIT_OK) {
                uBit.serial.printf("%d, ", addr);
            }
        }
        uBit.serial.printf("\r\n");
    }
    for (;;);
}

@finneyj is it part of the CODAL API that null (ie zero-length) byte writes are allowed on the I2C bus?  Well, it seems like it was on v1...

Well, it seems like it was on v1...

Actually that's not entirely true.  MicroPython on v1 never used the DAL, instead we wrote a custom function to probe the bus.  See i2c_probe() in https://github.com/bbcmicrobit/micropython/commit/51be2575976fa6a0d9f509df357740da348903c2

Closing as all these samples, including @dpgeorge 's without a 30us delay, work now with i2c and _i2c.


GradientTape.gradient fails when tf.gather is used after LSTM/GRU in tf.function

System information

Have I written custom code (as opposed to using a stock example script provided in TensorFlow): No
OS Platform and Distribution (e.g., Linux Ubuntu 16.04): Linux Stretch
Mobile device (e.g. iPhone 8, Pixel 2, Samsung Galaxy) if the issue happens on mobile device:
TensorFlow installed from (source or binary): binary
TensorFlow version (use command below): 2.1.0 and 2.0.0
Python version: 3.6

Describe the current behavior
Consider the following simple model running a GRU, tf.gather, and a linear regression:
import tensorflow as tf

inputs = tf.keras.layers.Input(shape=[None, 1], dtype=tf.float32)
hidden = tf.keras.layers.GRU(10)(inputs)
hidden = tf.gather(hidden, [0])
output = tf.keras.layers.Dense(1)(hidden)
model = tf.keras.Model(inputs=inputs, outputs=output)

@tf.function
def train(x, y):
    with tf.GradientTape() as tape:
        predictions = model(x, training=True)
        loss = tf.losses.mean_squared_error(y, predictions)
    gradients = tape.gradient(loss, model.trainable_variables)

train(tf.constant([[[1], [2], [3]]], dtype=tf.float32), tf.constant([[1]], dtype=tf.float32))

Both TF 2.0.0 and TF 2.1.0 fails, with a similar error message (the following is TF 2.1):
    ValueError: All inputs to `ConcreteFunction`s must be Tensors; on invocation of __backward_standard_gru_918, the 0-th input (IndexedSlices(indices=Tensor("Reshape_2:0", shape=(1,), dtype=int32), values=Tensor("Reshape_1:0", shape=(1, 10), dtype=float32), dense_shape=Tensor("Cast_2:0", shape=(2,), dtype=int32))) was not a Tensor.

Describe the expected behavior
The code should work.
Other info / logs
The problem is caused by tf.gather generating tf.IndexedSlices as a derivative. However, current LSTM/GRU are graph functions and the backprop algorithm for graph functions assumes the inputs must be tf.Tensors.
One possible workaround is not to use tf.function and use eager. Then the code works.
Another solution is to force conversion of the derivatives from tf.IndexedSlices to tf.Tensor. The easiest solution I found is to use * 1, which adds operation Mul to the computation graph, for which the conversion from tf.IndexedSlices to Tensor works. So the following works:
inputs = tf.keras.layers.Input(shape=[None, 1], dtype=tf.float32)
hidden = tf.keras.layers.GRU(10)(inputs)
hidden = tf.gather(hidden * 1, [0])
output = tf.keras.layers.Dense(1)(hidden)
model = tf.keras.Model(inputs=inputs, outputs=output)

@tf.function
def train(x, y):
    with tf.GradientTape() as tape:
        predictions = model(x, training=True)
        loss = tf.losses.mean_squared_error(y, predictions)
    gradients = tape.gradient(loss, model.trainable_variables)

train(tf.constant([[[1], [2], [3]]], dtype=tf.float32), tf.constant([[1]], dtype=tf.float32))

Possible solution
I assume an automatic conversion of tf.IndexedSlices to tf.Tensor should be performed. As a proof of concept, I changed the line https://github.com/tensorflow/tensorflow/blob/1f404dc482e3b7e4244cc1603725e555f06f8b9b/tensorflow/python/eager/function.py#L1731
to
  tensor_inputs.append(ops.convert_to_tensor(arg))

and the code then works as expected (converting the tf.IndexedSlices to tf.Tensor). However, a proper fix is definitely more complicated.

This is severe for performance, experiencing troubles as well!

Well, the * 1 on strategic places (as described above) makes the tf.functions work again and there is no slowdown (a proper fix will do a very similar thing from the performance point of view).
So what you can try is after every call to LSTM/GRU (or after an enclosing Bidirectional) write * 1, until the bug is fixed.

Well, the * 1 on strategic places (as described above) makes the tf.functions work again and there is no slowdown (a proper fix will do a very similar thing from the performance point of view).
So what you can try is after every call to LSTM/GRU (or after an enclosing Bidirectional) write * 1, until the bug is fixed.

Well I am using a significantly more complex model. It feels painful to do this, I am looking forward to the fix.

I keep running into this issue myself; it's a horrific regression relative to TF1.x, where no such problems existed.
It would be really great if the TF team would fix this in the next release, for example by auto-converting to a dense tensor on entry into the backwards function. Yes, there might be more performant options, but the current behaviour is simply breaking code.

Any updates on this?
Changing this line to
  tensor_inputs.append(ops.convert_to_tensor(arg))

solves the issue on 2.2.0-rc3 but just wondering if there is a proper change from TF team.

A gentle bump after another month -- still not working on current TF-nightly 2.3.0-dev20200602.

I've raised a similar issue  #31952 before, which got fixed. However that fix only works when the grads are calculated outside a graph ( i.e. in eager mode). It might shed some light on how to fix it in graph mode though. cc the author of that fix: @alextp

Can you send a PR changing the line and adding a test?

@alextp I can easily do it, but I am not sure it is a correct solution. Looking at the current master:
https://github.com/tensorflow/tensorflow/blob/9221044560b9ed34d60a4b07dc0895552d2540c5/tensorflow/python/eager/function.py#L1873-L1906
I assume it would make sense to test specifically for IndexedSlices only, and perform the same shape check as in the case of tf.Tensor? Hm, I think the best solution would be to do something like
      elif isinstance(arg, (ops.Tensor, ops.IndexedSlices)):
        if isinstance(arg, ops.IndexedSlices):
          tensor_inputs.append(ops.convert_to_tensor(arg))
        else
          tensor_inputs.append(arg)

and then continue with the shape-checking code.
Looking even deeper, maybe we should do the rewrite for the backward pass only? Here
https://github.com/tensorflow/tensorflow/blob/9221044560b9ed34d60a4b07dc0895552d2540c5/tensorflow/python/eager/function.py#L737-L747
the gradients in the backward pass are collected -- it would be enough to convert the IndexedSlices to Tensors here. So that the least number of silent conversions would happen (and the backward pass is the only problematic one).
Personally I would do the rewrite only for the backward pass -- what do you think @alextp? If you think it is fine, I will prepare the patch (including a test).

I have simplified the problematic input not to reference LSTM/GRU:
@tf.function
def summing_rnn(inputs):
  return tf.reduce_sum(inputs, axis=1)

@tf.function
def gradients(inputs):
  with tf.GradientTape() as tape:
    tape.watch(inputs)
    hidden = summing_rnn(inputs)
    hidden = tf.gather(hidden, tf.constant([0]))
    loss = tf.reduce_mean(hidden)
  return tape.gradient(loss, inputs)

gradients(tf.constant([[[1.0], [2.0]]])) # No error is raised

I am also creating a pull request.


Implement integration with "Mindful Eating - Fabric"

Mindful Eating - Fabric
Mindful Eating's goal is to rework Minecraft's food system in order to incentivize a balanced diet in a non-intrusive way. Integrating this mod with Farmer's Delight would ideally follow camcam_burger's (aka camcamcamcamcam, the original Forge mod author) design philosiphy described in his design log and his video design log.
camcam_burger admitted himself that the prospect of balancing and adding support for other food focused mods was too large of a task and decided to add a feature which allows implementation using data.

hasnt somebody already made an addon that does this? that mod should be data driven so given how popular FD is somebody must have done that before


Added templates to customize your milipede

Rebase from master & squash those commits together. :)

The branch cannot be merge.
And shouldn't body and bodyr be swapped ?

Fixed.

@evadot what do you mean swapped?

template['body'] is used if in reverse move and template['bodyr'] in normal mode.
For me bodyr means body-reverse.

Done!


1.408 doesn't work with --cert args

code-server version: 1.408-vsc1.32.0
OS Version: Ubuntu Server 18.04 LTS (VM on Hyper-V)

Description
Trying to run code-server with --cert and --cert-key arguments causes application to exit issuing error EEXIST: file already exists, mkdir '<cert-path>'.
Steps to Reproduce

Acquire certificates for HTTPS operation
Try to run code-server with certificates, e.g. ./code-server --cert fullchain.pem --cert-key privkey.pem

Error shown
Below is a complete example of the error I am getting when running code-server following above steps:
lucas@u1804-coder:~/code-server1.408-vsc1.32.0-linux-x64$ ./code-server --cert fullchain.pem --cert-key privkey.pem
ERROR { Error: EEXIST: file already exists, mkdir '/home/lucas/code-server1.408-vsc1.32.0-linux-x64/fullchain.pem'
  [stack]: 'Error: EEXIST: file already exists, mkdir \'/home/lucas/code-server1.408-vsc1.32.0-linux-x64/fullchain.pem\'',
  [message]: 'EEXIST: file already exists, mkdir \'/home/lucas/code-server1.408-vsc1.32.0-linux-x64/fullchain.pem\'',
  errno: -17,
  code: 'EEXIST',
  syscall: 'mkdir',
  path: '/home/lucas/code-server1.408-vsc1.32.0-linux-x64/fullchain.pem' }
free(): invalid pointer
Aborted (core dumped)

Previous versions
This error does not happen on previous code-server version 1.32.0-310.

Will fix. Thanks for the report!

Exact same issue. Is there an easy workaround? Thanks for posting Mentz.

The "workaround" I'm using is reverting to version 1.32.0-310.

same issue as #351

Sorry, I didn't think it was a duplicate. Seeing that it's a different error I thought it would be better to open a new issue for this one.

Got exact same error too, another workaround for now might be using reverse proxy like nginx to perform ssl transmission.

Did some testing and it looks like what fixed https://github.com/codercom/code-server/issues/351 also fixed this one (that fix hasn't been released yet though). The different arguments caused a different error message, but the root cause was the same.

same error, waiting to be fixed

Fixed with the latest release!


Fix docker ignore

Prior to this change on my system the seth-rpc image seemed to ignore the entire contents of rpc/src/messages, causing a build error, as mod.rs could not be found. This may only be an issue on docker 18.06.1-ce however, as it doesn't seem to reproduce on Jenkins.

If it helps any, here's the output of docker version and docker-compose version from my dev machine:
docker version:
Client:
 Version:           18.06.1-ce
 API version:       1.38
 Go version:        go1.10.3
 Git commit:        e68fc7a
 Built:             Tue Aug 21 17:24:51 2018
 OS/Arch:           linux/amd64
 Experimental:      false

Server:
 Engine:
  Version:          18.06.1-ce
  API version:      1.38 (minimum version 1.12)
  Go version:       go1.10.3
  Git commit:       e68fc7a
  Built:            Tue Aug 21 17:23:15 2018
  OS/Arch:          linux/amd64
  Experimental:     false

docker-compose version:
docker-compose version 1.17.1, build unknown
docker-py version: 2.5.1
CPython version: 2.7.15rc1
OpenSSL version: OpenSSL 1.1.0g  2 Nov 2017


Improve setup

Mostly improved the setup section, plus some other small editing tweaks.

Just wanted to say thanks for doing this!  Your documentation is a great help!

Just wanted to say thanks for doing this!  Your documentation is a great help!


script src fix

the script src in the docs "" is wrong. Under the install conditions it should be ""

This fix will come with our 1.8 release. Coming soon...


chore(legacy): look for 403s or 409s in add/leave tests to handle cloud-apps status code change

LGTM


Option to disable certificate validation

It would be nice to have the option to disable certificate validation. At the moment Isolario's SSL certificate expired and therefore the bgpreader fails to download these files. I know this isn't a bgpreader issue, but this option would be "nice to have".
$ bgpreader -w<PHONE_NUMBER>,1574218800 -t updates -d singlefile -o upd-file=https://www.isolario.it/Isolario_MRT_data/Taris/2019_11/updates.20191120.0300.bz2
HTTP ERROR: SSL peer certificate or SSH remote key was not OK (60)
2020-05-17 16:44:27 778855: bsdi_singlefile.c:146: ERROR: can't open file 'https://www.isolario.it/Isolario_MRT_data/Taris/2019_11/updates.20191120.0300.bz2'

Are there any options I can set myself to temporarily resolve this issue?
Thanks! :)

@cmosig you can try rebuild libwandio with CURLOPT_SSL_VERIFYPEER and CURLOPT_SSL_VERIFYHOST set to zero, and then rebuild libbgpstream.

https://github.com/wanduow/wandio/blob/33e94742a53e8b59a56b46f347107170888ccf2e/lib/ior-http.c#L299 -> curl_easy_setopt(DATA(io)->curl, CURLOPT_SSL_VERIFYPEER, 0L);
https://github.com/wanduow/wandio/blob/33e94742a53e8b59a56b46f347107170888ccf2e/lib/ior-http.c#L300 -> curl_easy_setopt(DATA(io)->curl, CURLOPT_SSL_VERIFYHOST, 0L);

Thanks a lot, this worked!


Pivotal integration exception

Hi,
I've added Pivotal integration and from time to time I receive those two e-mails when working in Pivotal:
I'm running ubuntu 20.04
If you need more information, I'd be happy to help.
Logger django.request, from module django.utils.log line 224:
Error generated by PivotalMessenger <pivotal-bot@***> (Member) on *** deployment

No stack trace available

Deployed code:
- git: None
- ZULIP_VERSION: 4.2

Request info:
- path: /api/v1/external/pivotal
- POST: {}
- REMOTE_ADDR: "<IP_ADDRESS>"
- QUERY_STRING: "api_key=******&stream=******&topic=******"
- SERVER_NAME: ""

Logger zerver.middleware.json_error_handler, from module zerver.middleware line 450:
Error generated by PivotalMessenger <pivotal-bot@***> (Member) on *** deployment

Traceback (most recent call last):
  File "/usr/lib/python3.8/xml/etree/ElementTree.py", line 1693, in feed
    self.parser.Parse(data, 0)
xml.parsers.expat.ExpatError: not well-formed (invalid token): line 1, column 0

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "./zerver/webhooks/pivotal/view.py", line 172, in api_pivotal_webhook
    subject, content = api_pivotal_webhook_v3(request, user_profile)
  File "./zerver/webhooks/pivotal/view.py", line 19, in api_pivotal_webhook_v3
    payload = xml_fromstring(request.body)
  File "/srv/zulip-venv-cache/9d0f5ac272f4e644b222ed65b0b5a996616a215f/zulip-py3-venv/lib/python3.8/site-packages/defusedxml/common.py", line 131, in fromstring
    parser.feed(text)
  File "/usr/lib/python3.8/xml/etree/ElementTree.py", line 1695, in feed
    self._raiseerror(v)
  File "/usr/lib/python3.8/xml/etree/ElementTree.py", line 1602, in _raiseerror
    raise err
  File "<string>", line None
xml.etree.ElementTree.ParseError: not well-formed (invalid token): line 1, column 0

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/srv/zulip-venv-cache/9d0f5ac272f4e644b222ed65b0b5a996616a215f/zulip-py3-venv/lib/python3.8/site-packages/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/srv/zulip-venv-cache/9d0f5ac272f4e644b222ed65b0b5a996616a215f/zulip-py3-venv/lib/python3.8/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File "./zerver/lib/request.py", line 390, in _wrapped_view_func
    return view_func(request, *args, **kwargs)
  File "./zerver/decorator.py", line 354, in _wrapped_func_arguments
    raise err
  File "./zerver/decorator.py", line 334, in _wrapped_func_arguments
    return view_func(request, user_profile, *args, **kwargs)
  File "./zerver/lib/request.py", line 390, in _wrapped_view_func
    return view_func(request, *args, **kwargs)
  File "./zerver/webhooks/pivotal/view.py", line 175, in api_pivotal_webhook
    subject, content = api_pivotal_webhook_v5(request, user_profile)
  File "./zerver/webhooks/pivotal/view.py", line 87, in api_pivotal_webhook_v5
    story_url = primary_resources["url"]
KeyError: 'url'

Deployed code:
- git: None
- ZULIP_VERSION: 4.2

Request info:
- path: /api/v1/external/pivotal
- POST: {}
- REMOTE_ADDR: "<IP_ADDRESS>"
- QUERY_STRING: "api_key=******&stream=******&topic=******"
- SERVER_NAME: ""

Thanks for the report @vrozkovec!  Error payloads from webhooks should be logged to /var/log/zulip/webhooks_errors.log; can you capture that to figure out what event type is causing the exception?
(Sharing the whole payload, potentially after anonymizing any data with example.com type values, is ideal for being able to fix it quickly, but even just knowing the event type makes it a lot easier to reproduce)

Hi @timabbott, here are few last payloads:
log.txt

@vrozkovec thanks!  That should make it possible for someone to investigate and fix this.

@zulipbot claim

@vrozkovec are you wanting to alert on this event or not? As we have a list of a bunch of unsupported events that we just ignore, we can just add to this if you aren't specifically wanting to alert on this event.

As the content of that payload seems pretty useless to be fair.

@adambirds looking at the log it relates to creating new labels / epics, right? I think this can be ignored, me personally I'm not interested in those events.

Yeah I agree we should ignore. #18598 will do this.


Dependabot/maven/complete/org.springframework.boot spring boot starter parent 3.0.0

@AdilusPrimus Please sign the Contributor License Agreement!
Click here to manually synchronize the status of this Pull Request.
See the FAQ for frequently asked questions.

Hi @AdilusPrimus .  Thanks for the PR!  There is a lot here I'll try to touch on briefly.  I see that you have deleted a few key files, such as the README.adoc, the initial folder, and support for gradle in the complete directory.  These are all integral to having the guide operational on spring.io.  Additionally we'd like to keep the project very simple, so additions specific to vscode, devcontainers, and docker are out of scope.  For the dependabot suggestion we'd like to keep the dependency updates manual at this time mainly because it allows us to accept contributions from first time contributors.
I will close this PR as the scope is too broad at this point, but I do welcome another PR based on this issue of upgrading the Spring Boot version.


Possibly make 'add continue button' label more explicit about its purpose

It could have the text 'All questions on this screen are opitonal'

Some practical reasons we have this:
It helps you give a more semantically meaningful item in the interview order block when the variables listed on the screen don't match the purpose of displaying the screen. It adds a custom variable to the interview order block rather than the first field on the screen.
You might do this if you want to go back in and add show/hide logic on that screen, which could otherwise break the interview flow.

For people who run the wizard the first time, this won't make much sense. Hard to figure out how and whether to explain what it does. But it does feel like a useful option to leave in place.

Lily's idea: "if you want to override the default logical flow"

Resolved in develop branch


Add support for importing MMIO capabilities reduced permissions.

This is most useful for read-only access, but the mechanism should be sufficiently general to permit other uses (in particular, once we have CHERI-aware devices, it may be useful to restrict the ability to store capabilities and the ability to load mutable capabilities from devices).

This is generally useful and has been on my to-do list for a while, but is specifically useful as a building block for #133.


Support Arbitrary mutual exclusion category names

As discussed in #580.

So using the following we can set up mutually exclusive operations with arbitrary categories?
let mutuallyExclusiveWithArbitraryCategory = TrueCondition(name: "Condition 1", mutuallyExclusiveCategory: "Condition 1")

Yes. You can do something like this:
let foo = Foo()
foo.add(condition: TrueCondition(mutuallyExclusiveCategory: "Red"))
let bar = Bar()
bar.add(condition: TrueCondition(mutuallyExclusiveCategory: "Red"))
let baz = Baz()
baz.add(condition: TrueCondition(mutuallyExclusiveCategory: "Green"))
let bat = Bat()
bat.add(condition: TrueCondition(mutuallyExclusiveCategory: "Green"))
bat.add(condition: TrueCondition(mutuallyExclusiveCategory: "Blue"))

@pronebird feedback on this would be much appreciated - especially when adding multiple exclusive conditions - the test coverage around those scenarios is probably a bit weak, so might want to bear that in mind.

@danthorpe cool. As of now I still maintain a dictionary of category => operations. I'll probably convert it to TrueCondition and let you know if I hit any issues.

Works on a very limited test case.
    func testSerialExecutionForSimilarRequests() {
        var counter = 0

        var expectations = [XCTestExpectation]()

        for i in 0..<50 {
            let expect = expectation(description: "Wait for fetch \(i)")

            expectations.append(expect)

            runOperationWithCategory("MyCategory") { getResult in
                let _ = try! getResult()

                DispatchQueue.main.async {
                    print("Finished query \(i)")

                    XCTAssertEqual(counter, i)
                    counter += 1

                    expectations[i].fulfill()
                }
            }
        }

        waitForExpectations(timeout: 20)
    }

Using the same setup as before I get this error:

Type of expression is ambiguous without more context

For this line of code:
clearCoreData.add(condition: CoreDataExclusive())

My CoreDataExclusive is defined as:
public enum CoreDataWork { }
public typealias CoreDataExclusive = MutuallyExclusive<CoreDataWork>

Any ideas why this error would show up?

@ryanjm: I am unable to reproduce that error using the latest development branch and Xcode 8.2.1 (8C1002). Could you try with development?

Sorry @swiftlyfalling, my fault. I forgot to change my clearCoreData from a BlockOperation to BlockProcedure.
Thank you for taking a look.

@ryanjm: Ah, yup - that would do it!


Add badges to README top

Badges
Add cool badges.
See https://shields.io/category/issue-tracking for full list.

Absolutely in favor of having this!
Maybe we can add also:

Licence and version tag
Closed/open issues; closed/open PRs
@t-kimber's customized badge: https://img.shields.io/static/v1?label=socool&message=TeachOpenCADD&color=<red>

@AndreaVolkamer could you please check if you are ok with the new badges: https://github.com/volkamerlab/teachopencadd/blob/5760e98b217e619ef2a730a12d9767abb78944da/README.md

@t-kimber and @dominiquesydow thanks both of you, they look great!
Two thoughts

not entirely sure about the 'soocool' badge, although TOC is soocool ;)
and while looking at the README, I was generally wondering if we should move a short intro sentence up, right below the header TeachOpenCADD. So the reader knows what to expect before the details including badges and new content follows (which on the laptop takes ~ half a page)

Thanks @AndreaVolkamer for your feedback !

For the TOC badge itself, I suggest 2 alternatives: either no label, or project label, see screenshot. Maybe a bit more professional? :wink:

Concerning clarity for the reader, I would not necessarily add a sentence in the README since it is shown directly in the "about" section (see screenshot) but it's not my decision to make. @dominiquesydow and @AndreaVolkamer , your call :)

@t-kimber thanks!

I do have a slight preference for the one with 'project'
How about we simply include the very same sentence as in 'About' below the TeachOpenCADD README title?

Done :)

Thank you, looks great to me! @t-kimber, this PR is ready to go from my side :)


Negative Specular

When the camera turns, somes faces have this "negative specular".
Reddit post

hey, how did you set this up in godot in the first place? do you have a discord or any way you could run me through your steps?

I'm not sure about how to fix this issue, I probably need to code my own lighting solution.

#4

Transforming the normals seemed to fix it. #4