[5.2] Allowed Memory size exhaused when accessing undefined index in toArray

Hey !
Succeed to reproduce and isolate a curious bug :
When I try to access an undefined index, it crash and throw "Allowed Memory size exhaused"
Here is the reproduceable bug :
public function toArray()     {         $buffer = parent::toArray();         dd($buffer['random']);         return $buffer;     }
Neither of isset() nor empty() worked for me
(PHP 7.0.1 / Debian 8.4)
Regards,
Robin

You are just running out of memory when calling parent::toArray(). You either need to reduce the amount of items in your collection or increase your allowed memory allocation.

No, it's a bug. I don't know if it's coming out of Laravel or PHP itself, but it's a bug, 100% positive.
This issue showed because I was treating pivot values while serializing a plain object. It should throw me an undefined index error.
To fix this issue, i had to ensure to never try to serialize this plain object, and only serialyze it as a relation.
As an example :
Post::where('user_id', '=', 1)->get()->toArray()
Doesn't work, but following does :
User::with('posts')->where('id', '=', 1)->get()->toArray()
Which is totally normal, but the espected error was undefined index not Allowed memory excedeed

I don't get it, what is this method?
public function toArray() { $buffer = parent::toArray(); dd($buffer['random']); return $buffer; }

@rchoffardet
Your examples are not the same thing.
Post::where('user_id', '=', 1) will return just one row.
While User::with('posts')->where('id', '=', 1)->get()->toArray() will run two queries and return multiple posts. Inflating your memory usage.
Try:
User::with(['posts' => function ($query) {
    $query->take(1);
}])->get()->toArray();

This should limit the number of posts you get. If you still get allowed memory exceeded then there might be something in your mutations that is causing too much nesting an thus exceeding your memory limits.

@themsaid :
Overriding toArray() method allow us to add custom feature before serialization, like cleaning/filter model or add some stuff. I often use it to format dates attributes to ISO 8601.
@TomKita :
Both lines of codes will return the same thing : all post belonging to the user with id equals 1
The difference is :
Post::where('user_id', '=', 1) will return a collection of Post object
User::with('posts')->where('id', '=', 1)` will return a collection of only one user and inside it a collection of posts
Both are almost the samen and memory usage is nearly the same.
(And FYI, I fixed it and use the second query everywhere, it just works fine, with no slow down nor intensive memory usage)
The problem is :
With the first option, pivot table's values aren't return and in my toArray() i try to access one of them. It should be throwing me an error undefined index not allowed memory exceeded.
Let's say that accessing an undefined attribute causes this effect because of my database design or stuff like that. Shouldn't isset() or !empty() (notice ! ) return me false instead of true ?
I do not know if that is a laravel bug or my specific version of php, and i'll try to reproduce it again. But this is clearly a bug.

Just tested with Post::find(1) and got a Allowed memory exhausted while accessing an undefined attribute. This shouldn't occur.

Sorry, just to clarify you did something like this Post::find(1)->undefined and got allowed memory exhausted?

No, i did Post::find(1)->toArray() and access an  undefined attribute inside the toArray() overrided method

If you remove the overridden method from your Post model and call Post::find(1)->toArray() do you still get the error?

No

I finally find my problem: Errors are correctly thrown and written in laravel log, although, as i'm building a private REST api, I was encoding exception in JSON, which may consume too much memory (maybe circular references ?).


Suppress external image warnings when building docs

Coverage remained the same at 94.454% when pulling baf1e7bac77a714c8d62266f6279df1eeb64105f on suppress_external_url_warnings into a21c9bcc700bc65c211aa021dc4baa49b22da68a on master.


Android Messages no longer loads

Bug description:

Android Messages now only produces a blank screen. It seemed like it started after AM displayed a message saying "A new version of Messages is available, please reload the page." or something along those lines
Steps to reproduce:

Just open Android Messages
Expected behavior
Android Messages to load
Screenshots

Environment:

Operating System: Windows 10

Ferdi Version: 5.6.0-beta.5 (x64)

Server: Ferdi

Debug information:

ferdi.json.txt

I just saw this as well.
This is my Ferdi info window:

Other information:

OS: Windows 10 Version 10.0.19042 Build 19042
Server: Franz

my full ferdi info:

Yep. It's dead:

so interestingly i switched over my my linux partition and it works fine. i'll note that my linux partition has a different version of everything:
Ferdi: 5.5.0 (x64)
Election: 8.2.3
Node.js: v12.13.0
System: Ubuntu 18.04.5

i just downgraded my Windows partition's installed version to the 5.5.0 release and can confirm Android Messages is working again.

Yeah, same here. I also downgraded after I saw your first note about it working and 5.5.0 is working for me, too.

So I needed to go back to 5.6.0 because I was having issues with Teams on 5.5.0.
So I'm back to Messages being broken. That said, I checked the Developer Tools and it looks like it's throwing errors based on attempting (and failing) to access the property _electron.webFrame.session.setSpellCheckerLanguages in the following line of spellchecker.js
    if ("darwin" !== t) try {
        debug("Trying to load dictionary", e), _electron.webFrame.session.setSpellCheckerLanguages([...e, "en-US"]), debug("Switched dictionary to", e), _isEnabled = !0
    } catch (e) {
        console.error(e)
    }

Full error is below:
    C:\Users\MasterCKO\AppData\Local\Programs\ferdi\resources\app.asar\webview\spellchecker.js:1 TypeError: Cannot read property 'setSpellCheckerLanguages' of undefined
    at switchDict (C:\Users\MasterCKO\AppData\Local\Programs\ferdi\resources\app.asar\webview\spellchecker.js:1)
    at _temp.update (C:\Users\MasterCKO\AppData\Local\Programs\ferdi\resources\app.asar\webview\recipe.js:1)
    at C:\Users\MasterCKO\AppData\Local\Programs\ferdi\resources\app.asar\webview\recipe.js:1
    at reactionRunner (C:\Users\MasterCKO\AppData\Local\Programs\ferdi\resources\app.asar\node_modules\mobx\lib\mobx.js:2097)
    at trackDerivedFunction (C:\Users\MasterCKO\AppData\Local\Programs\ferdi\resources\app.asar\node_modules\mobx\lib\mobx.js:759)
    at Reaction.track (C:\Users\MasterCKO\AppData\Local\Programs\ferdi\resources\app.asar\node_modules\mobx\lib\mobx.js:1787)
    at Reaction.onInvalidate (C:\Users\MasterCKO\AppData\Local\Programs\ferdi\resources\app.asar\node_modules\mobx\lib\mobx.js:2078)
    at Reaction.runReaction (C:\Users\MasterCKO\AppData\Local\Programs\ferdi\resources\app.asar\node_modules\mobx\lib\mobx.js:1753)
    at runReactionsHelper (C:\Users\MasterCKO\AppData\Local\Programs\ferdi\resources\app.asar\node_modules\mobx\lib\mobx.js:1890)
    at reactionScheduler (C:\Users\MasterCKO\AppData\Local\Programs\ferdi\resources\app.asar\node_modules\mobx\lib\mobx.js:1868)

Looking into it some more, the spellchecker.js I'm seeing in my currently installed build (5.6.0.nightly.15 build) is totally different from the one the latest commit in overall structure and in the specific call to session.setSpellCheckerLanguages. So I'm guessing this might be solved if the team can get another nightly build published.

I downloaded the latest nightly and it resolved the issue for me.

What is the information on your environment? I am still unable to use it on 5.6.0.nightly.15, which as far as I know is the latest build.

OS: Windows 10
Version: 10.0.19042 Build 19042
Ferdi About Window

OS: Mac OS X Big Sur 11.2.3 (20D91)
Ferdi About Window:

Huh, that's interesting that it works on Mac, but not windows...

Is there anyone who was able to get it working on Windows?

Still not loading for me on the latest nightly (win10).

Same thing here :) any w/a ?

Same thing for me :'(

I just reimaged my mac - and its not working. I'm also running the same Ferdi nightly version.
@kytwb @vantezzen - any help please?

Update: On my newly reimaged MBP (non M1), I manually installed the nightly (v15) and now, Android Messages works without any issues. Please try it out and get back with updates here so as to help the maintainers with new information.

I have this issue also, on Windows 10 Pro, running Ferdi 5.6.0.-beta.5 (x64)
fwiw I tried tinkering with spell check settings, since an earlier comment suggested that could be related, but nothing fixed it there.

@dylan-k - please check the recipes in this folder - most likely, you might have to replace your local webview.js and index.js from the above url into your local installation. Please try this after closing/quitting Ferdi first, applying the latest version and restarting Ferdi. If you want, you should take a backup of your older versions in case you want to rollback.
If the above works, then please be sure to upvote this feature request since it provides a user-centric (non-techie) way to address this problem for the future.

@michaeldrouin - please test the above - and if it works, please close this issue.

Did not work in my environment (5.6.0-nightly.15)

@michaeldrouin - please test the above - and if it works, please close this issue.

did not work on neither the nightly nor the beta. these files are exactly the same as what's in my local installation on both anyway.  currently still having to downgrade to the current stable release for it to work.

It started working for me again today on 5.6.0-nightly.15

It started working for me again today on 5.6.0-nightly.15

Same here

Yep, same here. I wonder if something changed on the Google side?

Yes it works for me now after updating to the latest nightly, 5.6.0-nightly.15

it works for me now on Windows 10 Pro, running Ferdi 5.6.0.-beta.5 (x64)

as others are saying, the problem seems to have fixed itself in both the beta and nightly. there must have been something with Google that temporarily broke it. closing the issue now.


Fix the const correctness of the DeviceTensor class.

The DeviceTensor code subverts the const correctness which can result in very difficult to find bugs.

PR
Author
Editor
Reviewers
Assignment
Approval
Merge

#1626
@YohannDudouit
@tzanio
@jakubcerveny + @v-dobrev
07/21/20
09/01/20
⌛due 09/08/20

The current definition of DeviceTensor does not violate const-correctness for the data: if you construct a DeviceTensor<const double> object from const double * you cannot change its entries -- the subscript operators (both () and []) return a reference to const double. What you can change potentially is the DeviceTensor object itself (e.g. to make it use another const double *), however, the class does not have any such methods at the moment. In this regard, the class is similar to a pointer -- the constness of the pointer is separate from the constness of the type it points to. I think this type of definition is quite suitable here since DeviceTensor serves as a view for externally managed data.
Also, note that you cannot construct DeviceTensor<double> from const double * unless you explicitly cast the pointer as double *. On the other hand, it is okay to construct DeviceTensor<const double> from double * as expected.

@v-dobrev The fact that it's correct in one case does not mean it's correct in general.
The accessor has the following signature: Scalar& operator(int... args) const, this is returning a non-const reference and the method claims that it is const for the class, this is a very dangerous thing to do. It means that we can modify const DeviceTensor, and the compiler will think that we did not. This can also result in almost impossible to track bugs, because when you allow to modify const objects you can't trust const any more.

It means that we can modify const DeviceTensor, and the compiler will think that we did not.

The compiler cannot (and I'm sure it will not) make such assumptions -- the contents of the pointer is not covered by the constness of the object -- just like with double * const pointer.
That said, I agree that allowing the data of an object const DeviceTensor<double> to be modified is not standard and takes a little getting used to -- you just need to remember to think of it as a double * const pointer replacement that supports multi-indexing.
In some regards, DeviceTensor (in its current form) is similar to the c++20 std::span class template (https://en.cppreference.com/w/cpp/container/span) where the indexing operator has similar signature -- it is a const method that returns a reference to the template type (https://en.cppreference.com/w/cpp/container/span/operator_at). Another similarity is that both classes do not own their data.
In the end, this is a design preference, so if others also prefer the change you propose, I'll be fine with it.

I'm very confused by what you are saying, if we have a class A,  almost everyone expect const A a to mean that you cannot change the state of a.

As I commented before, I don't see a good reason for this change -- the protection of the data is provided by the const qualifier in the Scalar template type. Additionally, the const-ness in the proposed modification is easy to break, due to the shallow copy used in the copy-constructor.
At this point, I personally prefer not to merge this. However, in a way, this PR already has two approvals (@YohannDudouit and @jakubcerveny), so my preference is overruled, unless @tzanio agrees with me. If that's the case then we'll need a 5th person to break the tie.

I think having bugs being caught by the compiler instead of having to find them with a debugger is a pretty strong argument.

@camiers here are the performance comparison on ex1 -pa -d cuda -m ../data/fichera.mesh -o 4:
On master:
            Type  Time(%)      Time     Calls       Avg       Min       Max  Name
 GPU activities:   46.51%  338.82ms       402  842.85us  757.82us  1.3410ms  _ZN4mfem10CuKernel3DIZNS_22SmemPADiffusionApply3DILi5ELi6EEEviRKNS_5ArrayIdEES5_RKNS_6VectorES8_RS6_iiEUliE_EEviT_
                    9.55%  69.604ms      1199  58.051us  56.799us  60.991us  _ZN4mfem10CuKernel1DIZNS_3addERKNS_6VectorEdS3_RS1_EUliE_EEviT_
                    8.94%  65.099ms       402  161.94us  160.10us  164.93us  _ZN4mfem10CuKernel1DIZNKS_18ElementRestriction13MultTransposeERKNS_6VectorERS2_EUliE_EEviT_
                    5.58%  40.640ms         1  40.640ms  40.640ms  40.640ms  _ZN4mfem10CuKernel1DIZNS_22QuadratureInterpolator6Eval3DILi3ELi0ELi0EEEviiRKNS_9DofToQuadERKNS_6VectorERS6_S9_S9_iEUliE_EEviT_

On this branch:
            Type  Time(%)      Time     Calls       Avg       Min       Max  Name
 GPU activities:   46.73%  339.43ms       402  844.35us  759.61us  1.3685ms  _ZN4mfem10CuKernel3DIZNS_22SmemPADiffusionApply3DILi5ELi6EEEviRKNS_5ArrayIdEES5_RKNS_6VectorES8_RS6_iiEUliE_EEviT_
                    9.57%  69.496ms      1199  57.961us  56.928us  59.936us  _ZN4mfem10CuKernel1DIZNS_3addERKNS_6VectorEdS3_RS1_EUliE_EEviT_
                    8.98%  65.260ms       402  162.34us  159.97us  166.50us  _ZN4mfem10CuKernel1DIZNKS_18ElementRestriction13MultTransposeERKNS_6VectorERS2_EUliE_EEviT_
                    5.19%  37.707ms         1  37.707ms  37.707ms  37.707ms  _ZN4mfem10CuKernel1DIZNS_22QuadratureInterpolator6Eval3DILi3ELi0ELi0EEEviiRKNS_9DofToQuadERKNS_6VectorERS6_S9_S9_iEUliE_EEviT_

Extending the review deadline to next week...

Merged in next for testing...

@v-dobrev can you please answer these questions:

Why do you consider that modifying the values inside a const DeviceTensor does not modify the DeviceTensor?
Why is catching bug at compilation instead of runtime not an argument to you?
Is there any advantage of the current design over the proposed one?
Is there any disadvantage of the proposed design over the current one?

Merged in next for testing...

There are a number of failures in tonight's autotest runs on tux429:
linalg/../general/forall.hpp(121): error: no instance of function "lambda [](int) mutable->void" matches the argument list and object (the object has type qualifiers that prevent a match)
            argument types are: (const int)
            object type is: const lambda [](int) mutable->void
          detected during:
            instantiation of "void mfem::RajaCudaWrap2D(int, DBODY &&, int, int, int) [with DBODY=lambda [](int) mutable->void &]"
(316): here
            instantiation of "void mfem::ForallWrap<DIM,DBODY,HBODY>(__nv_bool, int, DBODY &&, HBODY &&, int, int, int) [with DIM=1, DBODY=lambda [](int) mutable->void, HBODY=lambda [](int)->void]"
linalg/operator.cpp(429): here

linalg/../general/forall.hpp(140): error: no instance of function "lambda [](int) mutable->void" matches the argument list and object (the object has type qualifiers that prevent a match)
            argument types are: (const int)
            object type is: const lambda [](int) mutable->void
          detected during:
            instantiation of "void mfem::RajaCudaWrap3D(int, DBODY &&, int, int, int) [with DBODY=lambda [](int) mutable->void &]"
(319): here
            instantiation of "void mfem::ForallWrap<DIM,DBODY,HBODY>(__nv_bool, int, DBODY &&, HBODY &&, int, int, int) [with DIM=1, DBODY=lambda [](int) mutable->void, HBODY=lambda [](int)->void]"
linalg/operator.cpp(429): here

Re-merged in next.

@trws Thank you for sharing your opinion. The goal is to have const to mean that const DeviceTensor<T> is not modifiable, the problem is not about DeviceTensor<const T>.
If the Raja team is not interested to support mutable lambdas this is fine, this PR would make using the DeviceTensor less bug prone but it is not critical.

@trws Thank you for sharing your opinion. The goal is to have const to mean that const DeviceTensor<T> is not modifiable

I do see that that's how this PR is set up, and if that's what you and the project want then certainly do it. The reason I put this information here is that types like this, types with pointer-like rather than value-like semantics, do not normally work that way and may result in surprise.  Having it be based on the constness of the internal type makes it consistent with the stl, sycl, raja, boost, and others.

, the problem is not about DeviceTensor<const T>. Currently there is absolutely no difference between const DeviceTensor<T> and DeviceTensor<T>.

Right, and for a pointer-like type that's normal.
If the goal is something more like a smart reference or value type, there are a couple of ways to handle it. One is what you have here, another is to do this same thing, but when mutability is desires use an explicitly mutable wrapper around, or version of, the DeviceTensor.  If that's something you're interested in I'd be happy to help put it together.

If the Raja team is not interested to support mutable lambdas this is completely fine, this PR would make using the DeviceTensor less bug prone but it is not critical at all, we can revert the merge.

We support them in forall currently, but for some reason not in kernel. I meant what I said in that we will discuss the reasoning and work out which side of that we'll be on. There are optimizations we can only apply if we keep the lambdas immutable, but at the same time we rely on interior mutability for things like reducers to work, so it becomes a tricky subject. Likely there will always be at least a subset of the interface that will work with mutable lambdas, right now all lambdas work with types that have interior mutability, but at some point in the future being able to go immutable would allow us to do things like use read-only memory that can have significant performance advantages in some backends.

@trws I agree with most of what you said and I really like your design idea. If I follow correctly, since I want Tensor to behave like a Vector or a DenseMatrix then it should own its data, and as @v-dobrev mentioned, we should have a deep-copy constructor:
template <int rank, typename T, typename Layout = DefaultLayout>
class Tensor
{
private:
   Memory<T> data;
   Layout layout;

public:
   Tensor(const &Tensor)
   {
      // Deep copy
   }

   template <typename... Idx>
   T& operator()(Idx... idx){
      data[ layout(idx...) ];
   }

   template <typename Idx...>
   const T& operator()(Idx... idx) const{
      return data[ layout(idx...) ];
   }
}

I added the concept of Layout that I really like in Raja, and it could be very interesting for optimizing specific kernel memory accesses.
When we want to access the Tensor on device with MFEM_FORALL, then we use these three functors (and three other to force host memory access HostRead, HostReadWrite, HostWrite):
template <int rank, typename T>
MutTensor<rank,T> Write(Tensor<rank,T> &){
   return MutTensor(T.Write(), T.layout);
}

template <int rank, typename T>
MutTensor<rank,T> ReadWrite(Tensor<rank,T> &){
   return MutTensor(T.ReadWrite(), T.layout);
}

template <int rank, typename T>
ConstTensor<rank,T> Read(Tensor<rank,T> &){
   return ConstTensor(T.Read(), T.layout);
}

where MutTensor and ConstTensor have the respective accessors:
template <int rank, typename T = double, typename Layout = DefaultLayout>
class MutTensor{
private:
  T* data;
  Layout layout;

public:
   MFEM_HOSTDEVICE
   MutTensor(const MutTensor &t){
      // shallow copy for moving "objects" between host and device
   }

   template <typename... Idx>
   T& MutTensor::operator()(Idx... idx) const{
      return data[ layout(idx...) ];
   }
}

template <int rank, typename T = double, typename Layout = DefaultLayout>
class ConstTensor{
private:
   const T* data;
   Layout layout;

public:
   MFEM_HOSTDEVICE
   ConstTensor(const ConstTensor &t){
      // shallow copy for moving "objects" between host and device
   }

   template <typename... Idx>
   const T& ConstTensor::operator()(Idx... idx) const{
      return data[ layout(idx...) ];
   }
}

like this we don't need to use mutable lambdas and we have a cleaner and safer code.
Feel free to comment, criticize and propose ideas.

That sounds really good.  It's more noisy, you have to write more in the code than you would without the accessors, but it gives the user and the implementation potentially a lot more information, which could be really good.  For example you could have the ConstTensor put the data in read-mostly or read-only memory to get the extra caching layers in CUDA, which can be a nice win when it's possible.
One danger with this is that whenever the Tensor type is passed by value, or captured by value in a lambda, it will be deep copied, and while that's not automatically a bad thing it might be easy to miss.  Something that I personally like to do to help prevent cases like that, is you can make it move only with an explicit deep copy method rather than an implicit one.  That could look something like this:
template <int rank, typename T = double, typename Layout = DefaultLayout>
class Tensor
{
private:
   Memory<T> data;
   Layout layout;

public:
   Tensor(Tensor const &) = delete; // never copy silently
   Tensor(Tensor &&) = default; // move is fine, makes returning one cheap
   Tensor clone(const &Tensor)
   {
      // Deep copy
   }
...
}

If a user makes use of your accessors as references, that will also work just fine.  In fact making one of them an argument to a function should implicitly construct the accessor from a basic Tensor as long as the Const and Mut version's constructors are left non-explicit.  For me this would make for a really nice interface, if it wouldn't be too invasive in your code.

That sounds really good. It's more noisy, you have to write more in the code than you would without the accessors, but it gives the user and the implementation potentially a lot more information, which could be really good. For example you could have the ConstTensor put the data in read-mostly or read-only memory to get the extra caching layers in CUDA, which can be a nice win when it's possible.

I really like the idea to use the different types of memory on GPU, by read mostly do you mean Texture memory, and for read-only constant memory? Also do you know how important __restrict__ is for GPU performance? Using more efficiently cached object could be very interesting to us, because we have the same ConstTensor<2> B that we use in all threads, therefore any performance improvement to cache B is welcomed.
Does that mean that we should introduce another template parameter that specifies the "container"? It could be device memory, __shared__ memory, texture memory, constant memory, or register.
In order to use registers, and unroll loops efficiently I was writing a FixedSizeTensor class in this branch:
template<typename T, int... Dims>
class Tensor{
private:
   MFEM_SHARED T data[Size<Dims...>::val];
}

Do you think it would be possible to unify this with what we wrote by having something like StaticSharedContainer:
template <typename T, int... Dims>
class StaticSharedContainer{
private:
MFEM_SHARED T data[Prod<Dims..>::val];
public:
   T& operator[](const int i) const {
      return data[ i ];
   }
}

We could instead add a type of memory for StaticContainer, for more generality StaticContainer<Shared,T,Dims...>.
We would define also StaticLayout:
template <int... Dims>
class StaticLayout{
public:
   template <Idx... idx>
   usize operator()(Idx... idx){
      mfem_assert(sizeof...(Dims)==sizeof(Idx), "Wrong number of arguments.");
      return StaticTensorIndex<Dims...>::eval(idx...);
   }
}

And then doing some using to define convenient types:
template <typename T, int... Dims>
using SharedStaticMutTensor = MutTensor<sizeof...(Dims...),T,StaticLayout<Dims..>,StaticSharedContainer<T,Dims...>>;
template <typename T, int... Dims>
using SharedStaticConstTensor = ConstTensor<sizeof...(Dims...),T,StaticLayout<Dims...>,StaticSharedContainer<T,Dims...>>;

One danger with this is that whenever the Tensor type is passed by value, or captured by value in a lambda, it will be deep copied, and while that's not automatically a bad thing it might be easy to miss. Something that I personally like to do to help prevent cases like that, is you can make it move only with an explicit deep copy method rather than an implicit one.

I agree with you, but I think that for a math object it is okay to have deep copy, also we're protected from moving these on GPU by mistake because the copy constructor is not MFEM_HOST_DEVICE (which is the main lambda that capture by values for us). The Tensor class is meant to be used only on CPU, where we capture values by reference (or we should at least). But I don't really have strong feeling about it, and I see why what you propose adds safety.

That could look something like this:
template <int rank, typename T = double, typename Layout = DefaultLayout>
class Tensor
{
private:
   Memory<T> data;
   Layout layout;

public:
   Tensor(Tensor const &) = delete; // never copy silently
   Tensor(Tensor &&) = default; // move is fine, makes returning one cheap
   Tensor clone(const &Tensor)
   {
      // Deep copy
   }
...
}

I'm a bit outdated regarding move semantics in C++, I know move semantics from Rust, where it doesn't move anything (the move is resolved at compilation time). In C++, does it provide similar performance as RVO or NRVO? I was having the impression that move semantics is a better version of shallow copy.

If a user makes use of your accessors as references, that will also work just fine. In fact making one of them an argument to a function should implicitly construct the accessor from a basic Tensor as long as the Const and Mut version's constructors are left non-explicit.

Are you saying that I should do some automatic casting operators from Tensor to MutTensor and ConstTensor? That sounds like a good idea, that could be very powerful and useful, what kind of use for this did you have in mind?
I should probably define one from MutTensor to ConstTensor too, to be able to pass a MutTensor to a function that expects a ConstTensor.

For me this would make for a really nice interface, if it wouldn't be too invasive in your code.

Thank you for your help, this is super valuable.

That sounds really good. It's more noisy, you have to write more in the code than you would without the accessors, but it gives the user and the implementation potentially a lot more information, which could be really good. For example you could have the ConstTensor put the data in read-mostly or read-only memory to get the extra caching layers in CUDA, which can be a nice win when it's possible.

I really like the idea to use the different types of memory on GPU, by read mostly do you mean Texture memory,

I meant the managed memory hint mode that allows the region to be replicated into read-only memory on the device.

and for read-only constant memory?

This one was read-only global memory.  It's kinda like what Texture memory used to be, but not z-ordered.  Constant memory would be even better, but because of how it works it's hard to manipulate in an interface like this, something you could certainly look into though especially if you have small constant tables.

Also do you know how important __restrict__ is for GPU performance?

Less than it is for the host, ironically, because the programming model assumes both parallelism and unsequenced execution, so it only matters within a thread.  It's still important, but the impact is smaller.

Using
more efficiently cached object could be very interesting to us, because we
have the same ConstTensor<2> B that we use in all threads, therefore any
performance improvement to cache B is welcomed.
Does that mean that we should introduce another template parameter that
specifies the "container"? It could be device memory, __shared__ memory,
texture memory, constant memory, or register.
In order to use registers, and unroll loops efficiently I was writing a
FixedSizeTensor class in this
branch:
template<typename T, int... Dims>
class Tensor{
private:
   MFEM_SHARED T data[Size<Dims...>::val];
}

Do you think it would be possible to unify this with what we wrote by having something like StaticSharedContainer:

I don't see why not.  You could have a fixed-length view, or similar that would work with any input memory and guarantee the interior of a given size.  Actually having a class with a shared member wont work, unfortunately, but you could have it as a view that accepts some shared memory or gets it from a static or similar and have the same effect.

template <typename T, int... Dims>
class StaticSharedContainer{
private:
MFEM_SHARED T data[Prod<Dims...>::val];
public:
   T& operator[](const int i) const {
      return data[ i ];
   }
}

We could instead add a type of memory for StaticContainer, for more generality StaticContainer<Shared,T,Dims...>.
We would define also StaticLayout:

We actually have a layout somewhat like this for raja, it helps a great deal in some cases when you know at least some dimensions, even if you don't know all of them.

template <int... Dims>
class StaticLayout{
public:
   template <Idx... idx>
   usize operator()(Idx... idx){
      mfem_assert(sizeof...(Dims)==sizeof...(Idx), "Wrong number of arguments.");
      return StaticTensorIndex<Dims...>::eval(idx...);
   }
}

And then doing some using to define convenient types:
template <typename T, int... Dims>
using SharedStaticMutTensor = MutTensor<sizeof...(Dims...),T,StaticLayout<Dims..>,StaticSharedContainer<T,Dims...>>;
template <typename T, int... Dims>
using SharedStaticConstTensor = ConstTensor<sizeof...(Dims...),T,StaticLayout<Dims...>,StaticSharedContainer<T,Dims...>>;

One danger with this is that whenever the Tensor type is passed by value, or captured by value in a lambda, it will be deep copied, and while that's not automatically a bad thing it might be easy to miss. Something that I personally like to do to help prevent cases like that, is you can make it move only with an explicit deep copy method rather than an implicit one.

I agree with you, but I think that for a math object it is okay to have deep copy, also we're protected from moving these on GPU by mistake because the copy constructor is not MFEM_HOST_DEVICE (which is the main lambda that capture by values for us). The Tensor class is meant to be used only on CPU, where we capture values by reference (or we should at least). But I don't really have strong feeling about it, and I see why what you propose adds safety.

That could look something like this:
template <int rank, typename T = double, typename Layout = DefaultLayout>
class Tensor
{
private:
   Memory<T> data;
   Layout layout;

public:
   Tensor(Tensor const &) = delete; // never copy silently
   Tensor(Tensor &&) = default; // move is fine, makes returning one cheap
   Tensor clone(const &Tensor)
   {
      // Deep copy
   }
...
}

I'm a bit outdated regarding move semantics in C++, I know move semantics
from Rust, where it doesn't move anything (the move is resolved at compilation
time). In C++, does it provide similar performance as RVO or NRVO? I was
having the impression that move semantics is a better version of shallow copy.

Move in C++ is similar to move in rust when RVO is not possible.  For a vector,
for example, it will move the pointer, size and capacity and zero out the
original.  In fact, the end result would be that Tensor would work a lot like a rust Vec, though lacking the borrow checker.  Even so though, most compilers will at least warn on a moved-from value, so you get at least some of that protection.
The main advantage here though is not so much move itself, as the
fact that you can make something movable but not copyable.  That means if you
want to move one around, you can do so conveniently, and you only need to take
special action when it actually has to be a copy, and the compiler will error
out if you try to copy it in this case.  This is the mechanism that lets
unique_ptr work for example, ensuring that there is only ever one owner of that
object.

If a user makes use of your accessors as references, that will also work
just fine. In fact making one of them an argument to a function should
implicitly construct the accessor from a basic Tensor as long as the Const
and Mut version's constructors are left non-explicit.

Are you saying that I should do some automatic casting operators from Tensor
to MutTensor and ConstTensor? That sounds like a good idea, that could be
very powerful and useful, what kind of use for this did you have in mind?
I should probably define one from MutTensor to ConstTensor too, to be able
to pass a MutTensor to a function that expects a ConstTensor.

You already have actually, at least the first part.  The constructor from
Tensor& will give you that automatically for things like function arguments
since it doesn't have the explicit keyword.  The compiler should take care of
the coercion for you as long as the function is taking the accessor either by
value or const reference.
Given your existing mock-ups, I would expect this to work without issue:
void foo(ConstTensor t);
void bar(Tensor<...>&t) {
  foo(t);
}

Adding one to go from Mut to Const sounds like a good idea, though how that will
work with the data motion probably needs some careful thought.

Un-merged from next. A new implementation will be developed for this feature.

I meant the managed memory hint mode that allows the region to be replicated into read-only memory on the device.

I think I don't know what you are referring to, do you have some keywords I could use to find documentation, or do you have a reference for this?

and for read-only constant memory?

This one was read-only global memory. It's kinda like what Texture memory used to be, but not z-ordered. Constant memory would be even better, but because of how it works it's hard to manipulate in an interface like this, something you could certainly look into though especially if you have small constant tables.

Does a maximum of 256 double is a small table? Also, is it okay if the access pattern is different for each thread? (same for all blocks though). I had some really good results in the past using constant memory but it was somehow capricious to get consistent performance improvement.

We actually have a layout somewhat like this for raja, it helps a great deal in some cases when you know at least some dimensions, even if you don't know all of them.

That sounds really nice, I'll check it to see if it could be useful to us.

Move in C++ is similar to move in rust when RVO is not possible. For a vector,
for example, it will move the pointer, size and capacity and zero out the
original. In fact, the end result would be that Tensor would work a lot like a rust Vec, though lacking the borrow checker. Even so though, most compilers will at least warn on a moved-from value, so you get at least some of that protection.
The main advantage here though is not so much move itself, as the
fact that you can make something movable but not copyable. That means if you
want to move one around, you can do so conveniently, and you only need to take
special action when it actually has to be a copy, and the compiler will error
out if you try to copy it in this case. This is the mechanism that lets
unique_ptr work for example, ensuring that there is only ever one owner of that
object.

That makes sense, thank you for the explanation. When I was trying to use the move semantics with my FixedSizeTensor, because the data is a fixed size array, I was having the impression that the move semantics couldn't achieve what I wanted, and that only NRVO and RVO could, but maybe that I was just not understanding well enough the move semantics.
Thank you for all this valuable information, I will start implementing all this in a PR, I'll ping you if I'm struggling with some design aspects if that's okay with you. I don't want to abuse of your time, so feel free to tell me if I solicit you too much.

I think I don't know what you are referring to, do you have some keywords I could use to find documentation, or do you have a reference for this?

Sure, it's one of the options from cudaMemAdvise that tells the runtime to allow memory to be replicated rather than migrated.

Does a maximum of 256 double is a small table?

Definitely, that's a perfect candidate actually.  Constant memory is limited to 64k, so anything less should be ok.

Also, is it okay if the access pattern is different for each thread? (same for all blocks though). I had some really good results in the past using constant memory but it was somehow capricious to get consistent performance improvement.

It will matter, but less than with almost any other kind of memory, the whole of constant memory is cached into each SM.

That makes sense, thank you for the explanation. When I was trying to use the move semantics with my FixedSizeTensor, because the data is a fixed size array, I was having the impression that the move semantics couldn't achieve what I wanted, and that only NRVO and RVO could, but maybe that I was just not understanding well enough the move semantics.

You're probably right if the data is stored as a member of the object, but if it's stored as a pointer to a fixed-size buffer on the heap then it will certainly help.  Move only really helps if the object holds any large data through references or pointers, where RVO helps regardless, but only in case of return values where move can help any time you end up with an rvalue.

Thank you for all this valuable information, I will start implementing all this in a PR, I'll ping you if I'm struggling with some design aspects if that's okay with you. I don't want to abuse of your time, so feel free to tell me if I solicit you too much.

Sounds good, feel free to ping me.  I may be a bit slower to respond over the coming couple of weeks but I'm happy to help.

An attempt to implement the ideas discussed here is in the PR: #2626


I would think clothing would protect you from burning rather than being a lethal combination.  It's unintuitive and its heat retention should only turn on when you're below a temperature that hurts the player.
Reproduction

wear winter coat

burn(in my example I touched the corner of an atmos burn chamber)

immediately take a few seconds to extinguish

continue to cook due to coat heat retention

crit

continue to take heat damage

reach medbay with 800 burn damage for a fire that was extinguished minutes ago

The same thing happens if you get into a spaced corridor, start freezing to death, and then put on a winter coat in an attempt to stave off further cold damage

no issues. winter coat now make it easier to gain temperature and harder to lose temperature, they no longer modifier temperature thresholds

This isn't super intuitive, yeah. I've seen a lot of people ash - or get very close to ashing - because nobody around them connected the dots that their winter coat might be preventing them from cooling down.
Maybe fully removing this functionality would be a bit extreme - in part because it'd just make them a safe way to avoid getting cold, instead of a potential risk in event of a gas fire - but it could benefit from being toned down a little. It feels odd that a single, perfectly normal coat can make the difference between being readily revivable, and fully ashing.

We are talking about a cloth jacket that doesn't even cover the whole body, not a freakin Brazen Bull


Unable to lazy load modules - Module cannot be found error

Hello, I've created a Github repo here: https://github.com/mpalmer-sps/PluralSightNg2Fundamentals
I'm trying to add a module using a lazy load implementation (learning this) but the Angular documented approach does not work despite the syntax being correct. I'm uncertain if this is (maybe) not supported by the SPATemplates?
The error is the following:
ERROR Error: Uncaught (in promise): Error: Cannot find module 'app/user/user.module'. webpackEmptyContext@http://localhost:3258
The user.module is referenced here in the app.module:
{ path: 'user', loadChildren:'app/user/user.module#UsersModule'},
however, this doesn't work even when trying different path strings.
The workaround for this is to modify the navigation link in the navbar.component.html file to the following:
 Welcome User
Whereas before it was the following:
 Welcome User
and then removing the path with the loadChildren syntax and adding the UserModule as shown in the app.module.shared.ts file
Is this is expected behavior when using modules other than the app.module?

Since this is an Angular usage question, could you raise this with the Angular community or see what they have in docs? The templates here supply a typical Webpack+Angular starting point, but I'm afraid we don't have the capacity to also take on usage questions for third-party frameworks and packages.

I'm uncertain if this is (maybe) not supported by the SPATemplates?

The templates here don't affect what is or isn't supported, as they are not doing anything at runtime.
Hope that's OK!

Thanks @SteveSandersonMS ,
What this means is that, if I can't find an answer then I'm basically stuck.
The problem is that using the SPATemplates, with the file structure that is leveraged being so far outside of the 'norm' for Angular development, puts me in a no-man's land for support.
The Microsoft side of the house will point to the Angular side and say "the problem must be over there"..
Inversely, the Angular side of the house will take one look at the project structure and throw their hands in the air because the project structure with app.modules split into three files just to start is extremely difficult to trace where the actual issue sits.
In the end I may just simply have to abandon this approach of using Visual Studio 2017 and the SPATemplates because of that lack of support for what should be a basic Angular implementation in favor of something that is alot more "normal" in the Angular world.
Thanks

@SteveSandersonMS I've tried every possible option to load this Module leveraging the loadChildren method and it doesn't work. Would it be possible that you can provide a super simple example in a "Hello World" way where the loadChildren method is able to find the module?

Then make sure your module there has its routes set up correctly. Like in this example: https://github.com/MarkPieszak/aspnetcore-angular2-universal/blob/master/ClientApp/app/containers/lazy/lazy.module.ts
Hope that helps!

Thanks @MarkPieszak
Let me take a look at your solution and see what adjustments need to be made to mine to get this working. One a side note I see that you've stated in comments to look at the network tab to show the module/component being loaded but because everything is bundled into a larger package will this be a test that shows your lazy component being loaded?

@MarkPieszak
ok - update to this. Re-tried various path based approaches that mimic your setup but still no luck so I dug abit deeper.
Based on the Angular documentation here:
https://shlomiassaf.github.io/ng-router-loader/index.html
there is a line here where a Module is lazy loaded by referencing the module after it was already resolved with an import statement as in this example:
import { Routes } from '@angular/router';
import { DetailModule } from '../my-ng-modules/details';

then it was referenced in his code in this way:
export const ROUTES: Routes = [
{ path: 'detail', loadChildren: () => DetailModule },
];

So I took this and applied it to my users module in this way within app.module.shared:
import { UsersModule } from "./user/user.module";
and then added the route as shown below...
{ path: 'user', loadChildren: () => UsersModule },
This seemed to work but I wanted to test and be 100% certain that the module and components were not being loaded until I clicked the appropriate navlink.
Testing this with the network tab on FireFox and Chrome didn't work because of bundling of the JS files into a larger bundle (vendor.js).
So, I placed a pair of constructors in the User Module class and the Profile class that wrote to the console once they loaded and it looks like its working (at least there are no errors).
I'm still scratching my head as to why the syntax you have in your solution works and mine seems to fail, furthermore, I don't fully understand if this approach I've taken is "valid" and produces the benefits of lazy loading that I'm hoping to incorporate.

Comment out following line from webpack.config.js
// resolve: { mainFields: ['main'] }, // <-- comment this
Then lazy loaded chunks will be generated for AoT builds.

@davidsekar This saved my day. Thank you. @SteveSandersonMS is this a reason that "mainFields" line is in there? If not, it should likely be removed from the template.

@dloukola It was necessary at some point in the past to make server-side prerendering work correctly. If it's no longer necessary (perhaps third-party libraries have changed) then you can certainly remove it.
The next update to the Angular template eliminates webpack.config.js completely as it is based on the Angular CLI. So in effect this line will be removed, along with everything else from the file :)

@SteveSandersonMS This is great news about the removal of webpack. Do you have any estimate on when we can expect to see that go into the template?

Please see #1288 for full details. There isn't a specific ETA for release, but hopefully we will have a preview out in about a month.

@davidsekar I was able to get my 0-chunk.js file to generate after commenting that line of code out.   However, I do not see that file loading in the Network tab at all but yet my module still seems to run fine.
I added angular-router-loader since I am trying to preload this module.
Does the chunk get picked up by main-client and main-server?
Here is my webpack
const path = require('path');
const webpack = require('webpack');
const merge = require('webpack-merge');
const AotPlugin = require('@ngtools/webpack').AotPlugin;
const CheckerPlugin = require('awesome-typescript-loader').CheckerPlugin;

module.exports = (env) => {
    // Configuration in common to both client-side and server-side bundles
    const isDevBuild = !(env && env.prod);
    const sharedConfig = {
        stats: { modules: false },
        context: __dirname,
        resolve: { extensions: [ '.js', '.ts' ] },
        output: {
            filename: '[name].js',
            chunkFilename: '[name]-chunk.js',
            publicPath: 'dist/' // Webpack dev middleware, if enabled, handles requests for this URL prefix
        },
        module: {
            rules: [
                { test: /\.ts$/, include: /ClientApp/, use: isDevBuild ? ['awesome-typescript-loader?silent=true', 'angular-router-loader', 'angular2-template-loader'] : '@ngtools/webpack' },
                { test: /\.html$/, use: 'html-loader?minimize=false' },
                { test: /\.css$/, use: [ 'to-string-loader', isDevBuild ? 'css-loader' : 'css-loader?minimize' ] },
                { test: /\.(png|jpg|jpeg|gif|svg)$/, use: 'url-loader?limit=25000' }
            ]
        },
        plugins: [new CheckerPlugin()]
    };

    // Configuration for client-side bundle suitable for running in browsers
    const clientBundleOutputDir = './wwwroot/dist';
    const clientBundleConfig = merge(sharedConfig, {
        entry: { 'main-client': './ClientApp/boot.browser.ts' },
        output: { path: path.join(__dirname, clientBundleOutputDir) },
        plugins: [
            new webpack.DllReferencePlugin({
                context: __dirname,
                manifest: require('./wwwroot/dist/vendor-manifest.json')
            })
        ].concat(isDevBuild ? [
            // Plugins that apply in development builds only
            new webpack.SourceMapDevToolPlugin({
                filename: '[file].map', // Remove this line if you prefer inline source maps
                moduleFilenameTemplate: path.relative(clientBundleOutputDir, '[resourcePath]') // Point sourcemap entries to the original file locations on disk
            })
        ] : [
            // Plugins that apply in production builds only
            new webpack.optimize.UglifyJsPlugin(),
            new AotPlugin({
                tsConfigPath: './tsconfig.json',
                entryModule: path.join(__dirname, 'ClientApp/app/app.module.browser#AppModule'),
                exclude: ['./**/*.server.ts']
            })
        ])
    });

    // Configuration for server-side (prerendering) bundle suitable for running in Node
    const serverBundleConfig = merge(sharedConfig, {
        //resolve: { mainFields: ['main'] },
        entry: { 'main-server': './ClientApp/boot.server.ts' },
        plugins: [
            new webpack.DllReferencePlugin({
                context: __dirname,
                manifest: require('./ClientApp/dist/vendor-manifest.json'),
                sourceType: 'commonjs2',
                name: './vendor'
            })
        ].concat(isDevBuild ? [] : [
            // Plugins that apply in production builds only
            new AotPlugin({
                tsConfigPath: './tsconfig.json',
                entryModule: path.join(__dirname, 'ClientApp/app/app.module.server#AppModule'),
                exclude: ['./**/*.browser.ts']
            })
        ]),
        output: {
            libraryTarget: 'commonjs',
            path: path.join(__dirname, './ClientApp/dist')
        },
        target: 'node',
        devtool: 'inline-source-map'
    });

    return [clientBundleConfig, serverBundleConfig];
};

The angular documentation says loadChildren: 'app/customers/customers.module#CustomersModule'.
Changing the path to loadChildren: '../app/customers/customers.module#CustomersModule' worked for me.
Putting "../" before the "app"


retain_variables=True or addition in loss.backward()

In this line, you use retain_variables=True instead of adding the two loss terms for A3C and ICM, which is commented out a few lines later. Is there a difference?

There is no difference between them, as far as I know. You could use any of both, though I found none of them were effective in this repo


Any thoughts on a docker-alpine-oraclejre8 ?

I'm looking for the smallest possible image for running a single .jar web app server.
It feels like we could get this more minimal by removing javac and other parts not needed for just running jars.

Thanks for the suggestion! I agree that it makes sense to use JRE instead of JDK. There is develar/java image, but it is a bit outdated... I didn't want to duplicate the extraditing image, but now I see that my images are much popular and better maintained, so I may introduce JRE in the near future.


Make toolbar history work with placeholderfields

This is the flip-side to #3700. If the History menu remains in the toolbar at all times, then we need to document how users should integrate their custom apphook models that are editable from the front-end so that clicking 'Undo' actually impacts the model instance (e.g. blog post) being edited and not whichever Page the apphook is attached to.

This is not going to happen in 3.1, but definitely a much-needed improvements for django CMS

Closing as duplicate of https://github.com/divio/django-cms/issues/4920


scroll bar on the bottom is shown out in IE 11

When I access http://demos.creative-tim.com/light-bootstrap-dashboard/examples/notifications.html via my IE 11, the bottom scroll bar shown.
The 'log out' button on the top menu is out of the line.

Hello @rrongcheng, thank you for using our product;
please visit the following link: https://github.com/creativetimofficial/light-bootstrap-dashboard-pro/issues/49, I opened a new issue with this problem.
Hope this information helps you.
Greetings, Stefan


feat(spell): Implement Antimagic Field

Simple little AA-based spell, for the only thing part of said spell that can be reasonably automated using Foundry which is the immunity to damage from magic.
Saving throws are still rolled, and resulting effects still applied. Also, magic swords and the like do zero damage instead of being merely reduced to their mundane versions. Need a lot more support from upstream midi to implement things like that.
Still this covers 80% of what you want in combat from Antimagic Field and we're not trying to automate the DM making rulings about how every little thing interacts with a given spell.

I know that despite his life things Kandashi intends to update the module eventually, but it may be worth checking if it can be accomplished with Build A Bonus, which also supports auras. It's simpler than AA, so no macro-triggering on aura-entry/-departure, but it might be a better match here.
Also note to self/Giddy that it might be worthwhile to have options for this 'non-essential' behaviour? Like, AA isn't a dependency of PAA because you can plausibly run an entire campaign having never seen a single aura. So, if you have to use an aura, you might prefer to use a module you already have enabled rather than deal with redundancy.

Only addressing the Kandashi but, this is tested working with the v10 Mr. Primate fork, which is the de facto claimant to the AA throne while Kandashi is AFK

All good that the fork works fine; I'm just thinking more about efficiency (and a bit of longevity tbh). I never tested it properly but my anecdotal experience was that AA caused quite the slowdown while BAB seemed seamless.

AA definitely has some bugs that can cause performance to tank, but I don't think this effect can even be implemented with BAB, as my understanding is that BAB doesn't touch AEs. BABs bonuses are all stored in custom flags and, as the name suggests, add bonuses to various classes of dice rolls.
Since this spell provides complete immunity to magic, even magic that doesn't roll attack dice at all (ie Magic Missile), we can't rely strictly on manipulating roll data. We need to be able to set AE flags for Midi and I think AA is the only module that can do that with an aura. Happy to be proven wrong here.

Yes, worth at least documenting what this approach is and isn't capable of. It's not so much a ToDo because I can't think of a way to implement this even with a custom module, much less just item data.
You would need to crawl into the guts of the midi-workflow and even then it would be iffy. FVTT DND5e documents aren't structured in such a way to make this viable to ever completely automate. We don't break attack rolls into magical and non-magical parts.
Still, I much prefer not getting hit by a fireball to being engulfed in its firey embrace.

Yeah, all good. The _TODO field can certainly list TODOs that are never really going to get done 😄

_TODO'd, ready for merge


Transaction fee API - Error parsing json

Describe the problem you discovered
Take in charge these json structures:
getTransactionFee: responseHttp.body={"errors":{"content":["content size must be less than content_max_size"],"data":{"content":["content size must be less than content_max_size"]}},"status":"invalid"}

getTransactionFee: responseHttp.body={"errors":{"detail":"Internal Server Error"}}

Describe the solution you'd like
No response

fix here: https://github.com/archethic-foundation/libdart/pull/82


Search For Users

What does this PR do and why
This pull request introduces a feature that enables the search functionality for user profiles.
Screenshots or screen recordings

#954

@driesvints Hey, could you see why the test is failing?

Hello @driesvints please don't forget the review

Thanks a bunch for this one!


Suggestion for a better documentation from a newbie to React

I'm new to React and just started reading the documentation.
I noticed that references to the API are not links, just plain text. It would be helpful if they were actual links to the API.
Example: On this page https://reactjs.org/docs/rendering-elements.html it says:

The yellow API reference should actually be a link to the ReactDOM.render(), meaning a link to https://reactjs.org/docs/react-dom.html#render.
This helps me a lot because I can easily discover the complete reference of that function, the complete list and description of the parameters, function behaviors, different possible usage, etc.
As I go through the documentation and concepts, I can quickly go back and forth between concepts and API reference. It's much quicker to have links I can open in a new tab, instead of manually searching in the API reference every time.

Good suggestion! I know we do this in some places but we're not consistent about it.
Any interest in submitting one or more PRs to add links in places where it would be relevant?


FEAT: De-Duplicate GTFS-RT Ingestion Files

This change de-duplicates records for GTFS-RT parquet files.
To achieve de-duplication, the polars library is used in combination with a record-hash function.
This change also introduces a GTFS-RT parquet file partition column, which will partition parquet files by a specific gtfs-rt field.
Due to the compression performance of the parquet file format, this change is not guaranteed to reduce parquet file sizes on the scale of the 1 hour partitions we are currently writing. However, the opportunity to reduce the size of parquet files with larger partition spans is available.
Asana Task: https://app.asana.com/0/1205827492903547/1207039261301272

@mzappitello I removed the Glides try-except changes from this PR, can be implemented in a separate change.


Add recipe of beverage jug

About the PR

Added autolathe recipe for beverage jug. Cost 1 sheet of steel.
Why / Balance

Sometimes, need more of these jugs, but you can only get empty ones from the restock booze, it's too expensive for two empty jugs. For one steel we get 150u, while a bucket for the same price is 250u.
Media

[X] I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Breaking changes

Changelog

:cl:

add: Added autolathe recipe for beverage jug.

inb4 maintainers complain

mfw 1 steel for the plastic drink jug

Wait,  he made of plastic...?

Indeed it is a plastic jug ( I should know I made it lol )
I based it off the jugs I saw working in the food industry a while

Wait a sec, do we not have fill level sprites for this clear, plastic jug?

It's uh, not clear

Ah, well okay then. Nevermind!


Publish VS Code plugin to the Open VSX Registry

Arch systems use the https://open-vsx.org/ marketplace and not the default Microsoft one. This means that the sway plugin cannot be found in the VS Code extensions. As a result, we should add support for publishing to the Open VSX Registry.
Users of Arch can instead find the plugin to install at https://aur.archlinux.org/packages/code-marketplace
related to #65

I wanted to check on this issue. Currently, users on Arch and a bunch of derivatives won't be able to find this extension by default. OVSX has a page on how to publish an extension to their registry here, though likely the easiest solution is integrating the GitHub Action that they link at the bottom here.


Making SQLConnectionManager's execute() more resilient

I'm working on building an adapter for our legacy MSSQL Server 2008 R2 box so we can untangle that box before migrating all of it off to Snowflake. pyodbc is Microsoft's suggested library for interacting with MSSQL, so I'm using that.
If you look at this line of the SQLConnectionManager, you'll see that the execute() statement is always called with the bindings variable, even if it's None.
Pyodbc doesn't like this behavior, so I have temporarily overloaded this method to make this single line
if bindings is None:
    cursor.execute(sql)
else:
    cursor.execute(sql, bindings)

Overloading the whole function seems unnecessary for such a small change, and ODBC is a common enough driver that I imagine this will be a repeated issue. I think it would be wise to add alter the cursor execution in the SQLConnectionManager.
As far as I can tell, this won't produce any ill effects to existing adapters.

@jacobm001 Thanks for the suggestion, this is a great idea! I guess so far we've been lucky that our adapters all accept bindings=None, but I have encountered other adapters with this issue so it must be a common pattern.
The big change here will probably be fixing any unit tests that expect bindings=None to get passed to a mock cursor.execute - I think there are only a handful in the test_*_adapter.py files.
If you're interested in opening a PR for this, the dev/louisa-may-alcott branch (v0.15.0-to-be, I think) would probably be the most appropriate.

Sure, I'll take a stab at it later this week.


Added error enum

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@4e36d9c). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master      #15   +/-   ##
=========================================
  Coverage          ?   50.00%
=========================================
  Files             ?        4
  Lines             ?      106
  Branches          ?        0
=========================================
  Hits              ?       53
  Misses            ?       53
  Partials          ?        0

Continue to review full report at Codecov.

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


Support soft assertions in Testerina

Description:
In the current Testerina test framework, we only support hard assertions. We need to support soft assertions as well. That is test function should be able to proceed with an assertion failure and throw the assertion exceptions at the end of the test function.
Steps to reproduce:
Affected Versions:
OS, DB, other environment details and versions:
Related Issues (optional):

Suggested Labels (optional):

Suggested Assignees (optional):

We can achieve this using trap. See the example below:
@test:Config {}
function testFuntion() returns error? {
    never|error var1 = trap test:assertFail();
    io:println("message after assert fail statement");
    return var1;
}


Error reading page

Hi,
Nice work on the plugin, it looks really promising but I'm having a problem getting it to work.
I believe I have followed the README.md correctly.
❯ mkdocs serve
INFO     -  Building documentation...
INFO     -  Cleaning site directory

Done! ✨
Check out your shiny new generated files at /tmp/tmpc3gqbk39.

ERROR    -  Error reading page 'index.md': [Errno 2] No such file or directory: '/tmp/tmpc3gqbk39/api.md'
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/tmpc3gqbk39/api.md'

The same error is returned by mkdocs build.
The files I have setup are:
❯ tree
.
├── docs
│   ├── api.yaml
│   └── index.md
└── mkdocs.yml

❯ less mkdocs.yml
site_name: MkDocs example
site_url: https://example.com/
nav:
  - HTTP: index.md
plugins:
  - render_asyncapi

❯ less docs/api.yaml
asyncapi: 2.5.0
info:
  title: Example API
  version: '1.0.0'
channels:
  example:
    subscribe:
      summary: Receive example event
      message:
        name: exampleevent
        payload:
          type: object
          properties:
            foo:
              type: string

❯ less docs/index.md
!!asyncapi api.yaml!!

I've tried running the asyncapi generator command directly and it seems to work, which is where the "Done" output seems to be coming from. Would appreciate your assistance!

Through debugging your library I found that the file outputted by asyncapi generate is always called asyncapi.md, and something requires this to be the same as the asyncapi YAML file for it all to work.


Automatic Hashable conformance

Discussion for automatic Hashable conformance. As we know, struct gets automatic synthesis to Hashable as a value type and this works nicely from https://github.com/CreateAPI/CreateAPI/pull/113 with defaultType: struct. We also know that class cannot be automatically conformed to Hashable since it has to manually implement Equatable as well.
Some issues are:

With defaultType: struct we implicitly convert to a class when it detects itself on a property type
A struct containing a property for a class (through a type override combination) would break

Due to these issues we just implement a single config option entites.includeStructHashable* for automatic struct Hashable conformance when it doesn't contain a property to a class. It seems like a long shot for us to also generate the necessary Equatable methods for a class, which I would generate using equality of all the properties but I also think this wouldn't be correct.
*differs from entities.includeIdentifiableConformance but I think we should rename that to entities.includeIdentifiable as the "conformance" part seems redundant and these names are lengthy.

With the work that's been implemented before this it isn't necessary since one can add conformance to entities.protocols if they generate as structs.


.ssh folder is not being mounted

Since using v0.16.11, my .ssh folder is not being mounted.  I can't connect to an external server without a password.  My .ssh folder is in my home folder.  It worked previously (v0.16.5 I think).
Boxfile
build:
  ruby_runtime: ruby-2.2

mysql1:
  name: my_db_development

/mnt is empty on my nanobox.


Emulation - percentHeight behaves differently

In Flex, setting height of 100% on object A does not necessarily mean it stretches to the size of its container B, whereas in Royale this is always the case.
In this example
<?xml version="1.0" encoding="utf-8"?>
<mx:Application
	xmlns:fx="http://ns.adobe.com/mxml/2009"
	xmlns:s="library://ns.apache.org/royale/spark"
	xmlns:mx="library://ns.apache.org/royale/mx"
	width="100%" height="100%">

    <fx:Script>
    <![CDATA[

    ]]>
    </fx:Script>
	<mx:HBox height="100%" width="100%">
		<mx:Box width="20%" height="100%">
			<mx:Canvas backgroundColor="red" width="100%" height="100%"/>
		</mx:Box>
		<mx:Box width="60%" height="100%">
			<mx:Canvas backgroundColor="green" width="100%" height="1800" />
		</mx:Box>
		<mx:Box width="20%" height="100%">
			<mx:Canvas backgroundColor="blue" width="100%" height="100%"/>
		</mx:Box>
	</mx:HBox>
</mx:Application>

The Flex example will show all canvases (red, green, and blue) stretching to a height of 1800 pixels, but in Royale, only the green canvas is sized to 1800 pixels, whereas the rest remain the size of the screen.

I will look into this, I have spent a great deal of time working on improving the emulation layouts (and will create a new branch for review soon). IIRC the Box layouts have a branch for handling a 'single' child situation (presumably optimized compared to the multiple children scenario) and I suspect the issue is in this code.

I looked into this.
I must say this Flex behavior does not make intuitive sense to me, and the Royale behavior seems more correct IMO. There are only top-down Container percent-based Container constraints that request "percentage of space in the parent". There is nothing here as a container that says to me: "size to content", so I would intuitively expect to see scrollbars on the Box containing the oversized Canvas. I'd be keen to see something somewhere that describes the intended behavior in Flex in this case. Maybe I just got used to how things are in Royale, but I don't recall dealing with this in Flex in the past (although, it has been a while...).
The reason that the Flex version of the BoxLayout 'breaks' out of the percent-based sizing is because of measuredMinHeight being 1800 at the HBox level and downwards during layout phase (similar to the issue we saw with measuredMinHeight in the PanelInternalContainer).
If you swap the mx:Box items to mx:Canvas items then the behavior in Flex is as it is currently in Royale with mx:Box. I don't really understand why a mx:Box with a single child is behaving so differently to a mx:Canvas with a single child in Flex (with the same percent-based sizing)... and whether this is intentional or is a bug.

It's been a while for me as well since I got into the docs relating to Flex's layout.
This excerpt from Flex_4.6_help.pdf may provide a clue
Determining container size
Flex uses the following basic rules, in addition to the basic component sizing rules, to determine the size of a container:
•
A percentage-based container size is advisory. Flex makes the container large enough to fit its children at their minimum sizes. For more information on percentage-based sizing, see
“Using percentage-based sizing” on
page
373
.


Update Set-RegulatoryComplianceUI.md

Current implementation for this cmdlet

Docs Build status updates of commit fbff192:
:clock10: Full build: the average full build time is 17 min(s) 41 sec(s), this is based on the last 1000 build(s) for this repository.

Docs Build status updates of commit fbff192:
:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

exchange/exchange-ps/exchange/Set-RegulatoryComplianceUI.md
:white_check_mark:Succeeded
View (exchange-ps)

For more details, please refer to the build report.
Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.
For any questions, please:Try searching in the Docs contributor and Admin GuideSee the frequently asked questionsPost your question in the Docs support channel

Docs Build status updates of commit 5e4885d:
:clock10: Full build: the average full build time is 17 min(s) 42 sec(s), this is based on the last 1000 build(s) for this repository.

Docs Build status updates of commit 5e4885d:
:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

exchange/exchange-ps/exchange/Set-RegulatoryComplianceUI.md
:white_check_mark:Succeeded
View (exchange-ps)

For more details, please refer to the build report.
Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.
For any questions, please:Try searching in the Docs contributor and Admin GuideSee the frequently asked questionsPost your question in the Docs support channel


gitstream job taking hours and hours to run, no information about why

Describe the bug
I made a change to my project's .cm/gitstream.cm file, and created a PR to try it out. I think it should be running dry run mode, but I can't tell. 17 hours later, the check still hasn't completed, and there is no log output to give me any clue about why.
The only feedback I see in the PR is this:
gitStream.cm / gitStream.cm
Started 16h 59m 5s ago
[View more details on gitStream.cm](https://docs.gitstream.cm/examples/)

Viewing more details doesn't really help, because it just leads me to the example document.

And there are no log messages at all to tell me what has or hasn't happened in the past 17 hours.

To Reproduce
First, here is my .cm file (with comments stripped, for brevity):
# -*- mode: yaml -*-

manifest:
  version: 1.0

automations:
  estimated_time_to_review:
    if:
      - true
    run:
      - action: add-label@v1
        args:
          label: "{{ calc.etr }} min review"
          color: {{ 'E94637' if (calc.etr >= 20) else ('FBBD10' if (calc.etr >= 5) else '36A853') }}

  safe_changes:
    if:
      - {{ is.formatting or is.docs or is.renovate_config }}
    run:
      - action: add-label@v1
        args:
          label: 'safe-changes'
      - action: approve@v1

  github_teams_changes:
    if:
      - {{ files | match(regex=r/\.github\/teams.yml/) | every }}
    run:
      - action: add-label@v1
        args:
          label: 'safe-changes'
      - action: approve@v1

calc:
  etr: {{ branch | estimatedReviewTime }}
is:
  formatting: {{ source.diff.files | isFormattingChange }}
  docs: {{ files | match(regex=r/\.md$|\.rst$/) | every }}
  renovate_config: {{ files | match(regex=r/renovate.json5/) | every }}

The PR URL: (removed for security reasons)
Describe your PR relevant content: This PR has two changes. First is the .cm file shown above. Second is adding a file called documentation/source/bar.md as a test.
Add relevant commit SHA: I don't know how this will help you, because it's not a public repo, but the commit SHA is 464acdb4b7db3788b45cd2518e05eb0dcc6df114
Expected behavior
I expected two things:

gitstream should approve the PR
gitstream should add dry run comments to the PR to explain what it's doing

Screenshots

Additional context
(I can't think of anything relevant)

Hi @ToddBradley this is an issue that happens due to GitHub API rate limit. Under the hood it means that gitStream service gets rejected when trying to interact with GitHub API too many time. We are wokring on resolving this, but it could take some time. On a prositive note, the GitHub action is not stuck or consume resources on your side, meaning it just looks busy - actually it is not running any more.

OK, thank you. What's your recommendation for working around this? I would like to see the dry run log to make sure my changes are working as I intended, before I commit the change to the .cm file. Should I just push an empty commit to my PR to get gitstream to run again?

Also, shouldn't the job be timing out after 5 minutes, instead of running forever? Our .github/workflow/gitstream.yml file says this:
jobs:
   gitStream:
     timeout-minutes: 5

As a workaround, since GitHub rate limit is calculated every 1 hour, you should wait 1 hour and then do another commit to tirigger gitStream like you said. The job is limited to 5 minutes, but it will fail much sooner when GitHub API will reutrn rate limit error, hence the GitHub action will stop execution. On your side it will look like the check hangs but actually it is just a spinner with no real resource behind it. We are working on a real fix to avoid this, once we do have it I will update this issue.
In the mean time, if you see the workarond is not good enough you can comment here so we can think of another in-between workaround.

Hi @elocke our aim is to have a solution in place by the end of this week. Thank you for your patience and understanding.

@elocke we are working on that, and fixed an issue that should help.

@elocke we are working on that, and fixed an issue that should help.

Any updates to this? We're still experiencing this issue daily

We did some improvements from gitStream side, but I understand that you're still experiencing issues with hitting the GitHub rate limit 😞
It's possible that there are certain factors specific to your repository that are causing it to hit the rate limit more frequently than other repositories. We will continue to investigate the issue and work on finding a solution, I hope to have better results the coming week.

Great news 🥳  we isolated the probelm and fixed the main reason for you getting rate limit.

Thanks for sharing the good news. Is there anything we need to do in order to take advantage of the fix, or is it all on your side?

You don't need to do nothing it was fixed on gitStream backend.

the issue with the job not timing out after timeout-minutes has passed?

No, but this should not happen any more.


Share artifacts between jobs

This PR shares the artifacts created during the build jobs with the e2e test jobs.
It cuts checks times from 21 min to 13 min.
Further improvements can be made by splitting the Ubuntu based jobs and Windows base jobs, since currently Windows builds are in the critical path.

Also noticed something interesting (not related to this PR):

Why are ubuntu 22.04 binaries significantly bigger than these in ubuntu 20.04?

Why are ubuntu 22.04 binaries significantly bigger than these in ubuntu 20.04?

They are not. WasmEdge binaries are significantly bigger than wasmtime.


Update database-commons, jdbc, mssqlserver, ... to 1.20.1

About this PR
📦 Updates

org.testcontainers:database-commons
org.testcontainers:jdbc
org.testcontainers:mssqlserver
org.testcontainers:mysql
org.testcontainers:oracle-xe
org.testcontainers:postgresql
org.testcontainers:testcontainers

from 1.18.3 to 1.20.1
📜 GitHub Release Notes - Version Diff
Usage
✅ Please merge!
I'll automatically update this PR to resolve conflicts as long as you don't change it yourself.
If you'd like to skip this version, you can just close this PR. If you have any feedback, just mention me in the comments below.
Configure Scala Steward for your repository with a .scala-steward.conf file.
Have a fantastic day writing Scala!

⚙ Adjust future updates
Add this to your .scala-steward.conf file to ignore future updates of this dependency:
updates.ignore = [ { groupId = "org.testcontainers" } ]

Or, add this to slow down future updates of this dependency:
dependencyOverrides = [{
  pullRequests = { frequency = "30 days" },
  dependency = { groupId = "org.testcontainers" }
}]

labels: test-library-update, early-semver-minor, semver-spec-minor, commit-count:1

Superseded by #1036.


Why isn't AdcMcp3008 in Nuget?

Why isn't this code packaged and released in Nuget?

Well, I compiled it and put it into Nuget since it looks to be a very long time since anyone updated things...
Here it is: https://www.nuget.org/packages/Microsoft.IoT.AdcMcp3008/


Add documentation for the door setup

Description

Add documentation of the newly refurbished door controller setup.
Issues

Discord bot is jank. Need to polish and upload source code.
Checklist

[X] I've labeled the PR appropriately.
[X] I've have built the website and verified that my changes work.
[X] I've linked the appropriate issues and related PRs.

UPDATE: Made new repo https://github.com/lugatuic/doorkeeper-driver as a meta repo (kinda). Tracking project specific stuff there. LMK if theres anything else other than the issues on that repo.
PS I will be squashing once i have the :white_check_mark:


Unable to parse command line options: Unrecognized option: --no-transfer-progress

Describe the bug
Unable to parse command line options: Unrecognized option: --no-transfer-progress
Subprocess exited with error 1
To Reproduce
cdk synth

Current directory is: /home/ec2-user/workspaces/enhanced-document-understanding-on-aws/source/lambda/layers/custom-java-sdk-config. Running build
Unable to parse command line options: Unrecognized option: --no-transfer-progress
usage: mvn [options] [<goal(s)>] [<phase(s)>]
Expected behavior
A clear and concise description of what you expected to happen.
Please complete the following information about the solution:

[ ] Version: [e.g. v1.0.0]

To get the version of the solution, you can look at the description of the created CloudFormation stack. For example, "(SO0281) - Enhanced Document Understanding on AWS. Version v1.0.0".

[ca-central-1 ] Region: [ca-central-1]

I am using cloud9 as the IDE.
uname -a
Linux ip-10-2-35-238.ca-central-1.compute.internal 4.14.322-244.536.amzn2.x86_64 #1 SMP Wed Aug 16 04:58:21 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Hi. Can you check which version of maven you have installed (mvn --version)? I am not seeing this error with the latest version (3.9.2).
It seems from your error message that you may not have a version of maven which supports the --no-transfer-progress flag, which was added in 3.6.1 (https://maven.apache.org/docs/3.6.1/release-notes.html#user-visible-changes).
Please try using the latest version of maven and running cdk synth again and see if your issue persists.

@twocoins-ca you can also remove that option, removing the option causes more verbose logging during build. Not using the option does not affect the artifacts generated from the cdk synth

@twocoins-ca, closing this issue. See responses above.


Fix static analysis issues

V3022 Expression 'objects == null' is always false. CollectTabViewModel.cs 134
V3022 Expression 'objects == null' is always false. CollectTabViewModel.cs 179
V3022 Expression 'objects == null' is always false. ProCollectTabViewModel.cs 146
V3022 Expression 'objects == null' is always false. ProCollectTabViewModel.cs 182
V3027 The variable 'point' was utilized in the logical expression before it was verified against null in the same logical expression. CollectTabViewModel.cs 474
V3004 The 'then' statement is equivalent to the 'else' statement. EditOutputCoordinateViewModel.cs 30
V3063 A part of conditional expression is always true if it is evaluated: ipSelectedObject != null. FeatureClassUtils.cs 107
V3083 Unsafe invocation of event 'PropertyChanged', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. NotificationObject.cs 38
V3137 The 'endIndexNeeded' variable is assigned but is not used until the end of the function. CoordinateMGRS.cs 220
V3063 A part of conditional expression is always false if it is evaluated: aiPoints == null. CollectTabViewModel.cs 252
V3137 The 'pFeatureRender' variable is assigned but is not used until the end of the function. CollectTabViewModel.cs 311
V3137 The 'pFeatureRender' variable is assigned but is not used until the end of the function. CollectTabViewModel.cs 325
V3137 The 'element' variable is assigned but is not used until the end of the function. ArcMapTabBaseViewModel.cs 119
V3137 The 'endIndexNeeded' variable is assigned but is not used until the end of the function. CoordinateGARS.cs 205
V3137 The 'endIndexNeeded' variable is assigned but is not used until the end of the function. CoordinateUTM.cs 218
V3137 The 'endIndexNeeded' variable is assigned but is not used until the end of the function. CoordinateDD.cs 297
V3063 A part of conditional expression is always false if it is evaluated: aiPoints == null. ProCollectTabViewModel.cs 229
V3137 The 'endIndexNeeded' variable is assigned but is not used until the end of the function. CoordinateDDM.cs 288
V3137 The 'endIndexNeeded' variable is assigned but is not used until the end of the function. CoordinateDMS.cs 335
V3008 The 'pFldEdt' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 336, 334. FeatureClassUtils.cs 336
V3024 An odd precise comparison. Consider using a comparison with defined precision: Math.Abs(A - B) < Epsilon. ExportCSV.cs 135
V3022 Expression 'sep != '\0'' is always false. ImportCSV.cs 86
Analysed using PVS-Studio as a part of pinguem.ru competition

@csmoore this is awesome that we have this pull request. At first glance it looks like a lot of the non needed code that has been removed in this pull request.


Test timeout

I noticed timeout parameter is not approximately equal to the time measured with the included test.
timeout=1 -> elapsed 4s
timeout=2 -> elapsed 6s
timeout=3 -> elapsed 8s
timeout=4-> elapsed 10s

Ok I spent some time debugging this: I was wrong when I told you that you need to send an explicit ping, it look like this is done internally right after connecting to the server.
So this explains why we have the "2x timeout" part of the final time.
What I still don't understand is why there's the "+ 2" part. Looking at strace I can see that there's a 2s sleep somewhere right after it fails to receive anything (you can see recvfrom returning after 1s with EAGAIN)
[pid 16177]      0.000134 sendto(5, "{\"jsonrpc\":\"2.0\",\"id\":0,\"method\":\"server.ping\",\"params\":[]}\n", 60, MSG_NOSIGNAL, NULL, 0) = 60
[pid 16177]      0.000197 recvfrom(5, 0x77d448000fc0, 8192, 0, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
[pid 16177]      1.047175 clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=2, tv_nsec=0}, 0x77d44faed480) = 0 (????)
[pid 16177]      2.000259 socket(AF_INET, SOCK_STREAM|SOCK_CLOEXEC, IPPROTO_IP) = 6

Ah, got it, that's the retry timeout:
https://github.com/bitcoindevkit/rust-electrum-client/blob/98e1bf205c609a75b0261c390f88e1a4cd8e390a/src/client.rs#L72
Since we push the error before sleeping, it will sleep for 1 << 1 so two seconds

This is pretty old, is it still something we need and should I target this for the next release?


Tagline is not working.

papaerback-demo set
Tagline = "Example site using gohugo Paperback theme"

But I could not get the expected effect on paperback-demo

The check in layout was not all lowercase, which should be used for lookups.
Pull Request #6 created.


add MTA metrocard one-time payment

MTA requires cvv and billing address for stored card payment. adding them as optional fields

@cookie223 Thank you for the contribution! Can you remove your details from sample_config.txt before I merge? I'm going to squash the merge so removing those details will prevent them from being forever stored in the Debbit git history.

Thank you so much for the reminder. I definitely shall remove them.  Please let me know when it's merged I will remove my branch

@cookie223 Merged


Broken flow in Neerpelt

Don't know which flow
2016-12-02 10:26:42 UTC
   (4 hour(s) ago)
xxx@gmail.com:be-neerpelt
Count: 46 - Version:

description:

errorMessage: Type: SyntaxError
Message: missing } after function body
Code:
function alert(message) {};var run = function(result){     // do some calculations     // return true or an error message     // return "Error occurred";     return true; }; try{ return {result: run(context_result, rogerthat), error: null }; } catch(ex){ return {result: null, error: ex };}

mobicageVersion:

occurenceCount: 46

platform: 4

platformVersion:

It was the "Schade" flow in different services of insurance companies


Image controller with interactors

Trello: https://trello.com/c/vf6xQW7w/739-prototype-images-controller-with-interactors-pattern
As discussed at our meeting last week this is a demo of image controllers implemented with the interactor gem. I'm opening this up for reviews, thoughts, forks and ideas about how we feel/move forward with this pattern.
This PR is relatively quickly done so may have some errors, it also hasn't made efforts to reduce code duplication.
There are two small changes made in this PR to the behaviour of the existing code:

We no longer preview on image creation - this always fails because of lack of alt text, so why bother trying?
Round the generation of crop_height rather than floor - this made tests slightly easier.

Here's a quick round up on my thoughts on the implementation here:
The good

The image controller is now comprehensible. Every method seems short enough that you can follow reading the code in one pass
The interactors offer opportunities to break down the business logic into easier to manage chunks
Once you know the structure it's pretty easy to map interactor to an action

The bad

Reading the controller there are no clues where you might find a class such as Images::Create
I got caught out a few times with the plural naming and accidentally prefixed interactors with Image::
I got in a bit of a muddle experimenting with the means to write the interactors - more on that later.

The ugly

Writing the tests was a bit strange. Since we rely on feature tests already I'm not sure if much value is provided by writing them now, they also felt a bit repetitive. However they do feel like they should be part of a future strategy.
There's quite a lot of code duplication in both the interactors and tests. We may need to use inheritance/concerns/helpers as a means to reduce.

Style to write the interactors
I quickly got myself into a muddle trying to work out the nicest way to write the interactors and how to deal with the big global context variable.
My first approach went for a very readable call method at the expense of the readability of the private methods: https://gist.github.com/kevindew/4ab20f0d861a83267395ff7d0c0d608f. In here you'll notice the various assignments to context and the frequent usage of context. In this way they're similar to our feature tests of using methods to describe a procedure.
In later approaches I started trying to use context less. I went more for the approach that we use delegators to access the context creation arguments (which in hindsight could be just instance variables) and then set a context at a point of completing or exiting. This felt more like we have a defined entry and exit state as opposed to collecting state as progress.
I'm interested in thoughts on which of the styles here is more appealing.

Anyway to wrap this up, please provide thoughts or ideas of things we can try to reduce any initial weaknesses with this approach. Even better would be to fork this branch and experiment with tweaks to it. Catch you in a week.

And now a rival PR (not really) - https://github.com/alphagov/content-publisher/pull/914

One thing that concerns me is the recent activity on this repo. I've raised an issue/question to get their thoughts on a couple of my comments below, but I notice that the issue responses are looking a little stagnant; that said, it's so un-opinionated that I can't see this being an issue

We could have a conditional of if result.error == :error_type_1 and not even check the failure? field since result.error would be nil if only errors used it.

but I wonder if we've thought about just rolling our own?

When it comes to rolling our own, I’m keen for us to try avoid that unless it becomes absolutely necessary. The reasons I feel this way are:

we’re not solving a bespoke problem, so if we need a bespoke solution we’re likely on a bad path
It’s a pattern to define/maintain/tweak/document - which can all be handled by a gem
It’s much harder to bikeshed on someone else’s decisions
The fact someone else’s stuff is domain agnostic makes it much harder to spill our domain in, as evidenced by command pattern in Publishing API which tried to define consistencies and then blocked re-usability.

One of the things I like about Interactor is that if we reach a point where we have to let it go because it's not right for our needs then we can just re-write an implementation for us. So mostly I'm keen for us to experiment with interactor until we feel that we've either customised it so much it's unrecognisable or that we're doing things with it outside it's purpose.

And now a rival PR (not really) - #914

Woop! nice one putting that together. I'm not really sure whether to comment here or there, but since there's already discussion here I'll comment here briefly, and move over there if it gets more in depth.
So I think I've identified two main differences in your suggestion with what we have with interactors: which are the distinct pre_op, op and post_op steps; and the ability to succeed or fail with a particular reason.
With regards to the distinct steps:
These feel a bit like before and after blocks in Rails terminology. Although it's been useful to talk in terms of post_op for sidekiq I think this pattern here is actually of low meaning due to it's generality. Most of our concerns seem to be if something is running in a lock or not, which I can imagine is where post_op (or after) can make sense.I think the difference of pre_op and op is a greyer area where there aren't defined bounds and it'd be much easier to read code that did the steps of those individually rather than bound into methods.
With regards to success failure reasons:
This does seem valuable - I wonder what ways we can tinker with it. I left a suggestion above in this comment about you can set a variable that'd otherwise be nil.
One of the things I think we lose once we tie things down to particular reasons for failure is the need to even be checking for failure?. E.g. if I take your example here: https://github.com/alphagov/content-publisher/blob/9f6e34b34a530039e5cf88505ed63cbe3479b30a/app/controllers/images_controller.rb#L14-L26 - as we're only checking that it failed for issues, then it will try run the success code even if it was marked as failed.

@kevindew thanks for the thorough comments :-). I think we both agree that the success/failure mechanism provided by interactor is the main thing that's not quite fit-for-purpose. I'm happy to go with your idea of if result.error == ..., or even just if result.issues.
The other change I made in my PR was to try and make working with the context easier, but I think your approach of not using it until we return is much easier to understand. Possibly I just created my own problem and solved it!
Otherwise, I'm still wondering if there's a way to expose the locking/transaction behaviour more clearly. The only idea I have so far is to actually put these calls inside the interactor, which does re-introduce duplication, but makes it easier to see what's going on when we have post op code, and that it's running outside of the lock/transaction.

Closing in preference of https://github.com/alphagov/content-publisher/pull/960


Regression in type inference with functional intersection type

TypeScript Version:  2.6.1 and 2.7.1
Code
declare var R: R.Static;
declare namespace R {
    interface Dictionary<T> {
        [index: string]: T;
    }

    interface FilterDict<T> {
      (obj: Dictionary<T>): Dictionary<T>;
    }

    interface FilterList<T> {
      (list: T[]): T[];
    }

    type Filter<T> = FilterList<T> & FilterDict<T>;

    interface Static {
        filter<T>(fn: (value: T) => boolean): Filter<T>;
        compose<V0, T1, T2, T3>(fn2: (x: T2) => T3, fn1: (x: T1) => T2, fn0: (x: V0) => T1): (x: V0) => T3;
        map<T, U>(fn: (x: T) => U): (list: T[]) => U[];
        prop<T>(p: string): (obj: any) => T;
    }
}

export const nonNullMapValues = R.compose(
  R.filter<string>(Boolean),
  R.map<any, string>(R.prop("1")), //error: Type 'string[]' is not assignable to type 'Dictionary<T>', Index signature is missing
  Array.from
);

Expected behavior:
Given the string type parameter specified on R.filter<string>(Boolean), the resulting functional type: Filter<string> must then satisfy the fn2 parameter typing in the R.compose signature. Because Filter<string> is an intersection type this means the T2 type parameter of R.compose must either be Dictionary<string> OR string[]. Given the string generic type parameter specified on R.map<any, string>(R.prop("1")) then the T2 type parameter can only be string[]. Because string[] matches with string[] OR Dictionary<T>, string[] is a valid input type for the functional Filter<string> intersection type. I fail to see why this should be a compilation error, and it wasn't one in typescript 2.5.3, so I believe this is a regression.
Actual behavior:
The typescript compiler tries to verify that string[] is a valid input for both function types on the Filter<string> intersection type. Fails because string[] is not compatible with Dictionary<string>, even though this check is unnecessary.

note to self. need to bisect to find which commit caused the change in behavior.

@mhegazy one thing to point out is that this scenario never worked with functional interfaces:
interface Filter<T> {
  (obj: Dictionary<T>): Dictionary<T>;
  (list: T[]): T[];
}

which should function identical to the intersection type:
interface FilterDict<T> {
  (obj: Dictionary<T>): Dictionary<T>;
}
interface FilterList<T> {
  (list: T[]): T[];
}
type Filter<T> = FilterList<T> & FilterDict<T>;

The intersection type worked with typescript 2.5.3 but then regressed in 2.6. It would be great if we could get it working with both functional intersection types and with functional interfaces.

I think the issue is that the typescript compiler is deciding that the functional intersection requires an input of Dictionary<T> & T[] where it should instead be Dictionary<T> | T[]. Same with the functional interface with more than one signature specified. The inputs themselves shouldn't be intersections, they are separate, and therefore should be unions. The function that satisfies the functional intersection can take in either of the input types, not an intersection of the two.

@mhegazy had anytime to look this over? Also could we get the issue labeled as a bug if appropriate?

Given the fairly involved repro and the length of time this has been open without change, this looks likely to be working as intended. If you can demonstrate this more clearly in a short repro please open a new issue, but we're not able to investigate lengthy repros. Thanks!


Support opensearch-sql:run and update developer_guide doc

Description

Support  opensearch-sql:run
Update developer_guide doc. add prometheus and filesystem description.
Add .java-version to .gitignore

Check List

[ ] New functionality includes testing.

[ ] All tests pass, including unit test, integration test and doctest

[ ] New functionality has been documented.

[ ] New functionality has javadoc added
[ ] New functionality has user manual doc added

[ ] Commits are signed per the DCO using --signoff

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

Codecov Report

Merging #1099 (e52d905) into 2.x (42143b9) will decrease coverage by 2.51%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##                2.x    #1099      +/-   ##
============================================
- Coverage     98.29%   95.78%   -2.52%
  Complexity     3465     3465
============================================
  Files           347      357      +10
  Lines          8645     9303     +658
  Branches        550      669     +119
============================================
+ Hits           8498     8911     +413
- Misses          142      334     +192
- Partials          5       58      +53

Flag
Coverage Δ

query-workbench
62.76% <ø> (?)

sql-engine
98.29% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

 Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.