gopass packaging status and possible improvements

gopass is being independently packaged and maintained for a lot of different platforms.
Most linux distributions ship some kind of gopass package and we're even available on MacOS (homebrew) and Windows (Scoop, Chocolatey).
Most packages are regularly updated (at least the rolling release / bleading edge ones) but some lack considerably.
Maybe some distributions are missing a (proper) gopass package.
I would like to get a better understanding about the packaging status in the different package managers and what we can do make sure that our packages are in a great shape.

Mandatory link wrt. the topic:
https://repology.org/project/gopass/versions
To be noted that the Debian one is not our Gopass but another one.

Thank you. I wanted to include that link in the issue.
Looking at the numbers in #1848 we can see that:

Most downloads are for our Linux AMD64 binary builds

Note: These binary builds are also used by the self-updater

The second largest downloads are for our Deb packages
Homebrew shows significant usage, too

Note: We might have many more users who obtain gopass through their package manager, but those usually don't publish usage numbers.
Homebrew
gopass has an official tap in homebrew-core, that should be mostly fine.
However it's not using make install because of some problems with the man page: https://github.com/Homebrew/homebrew-core/pull/73247. We should try to fix that. Using make install sets some build flags that would be nice to have. Filed #1852
Debian
Also: Ubuntu, Deepin, Devuan, Kali Linux, Pardus, Parrot, Raspbian
The situation of gopass on Debian-based distributions is very sad.
There is a gopass package available on these platforms and it's a pass-compatible implementation but it's not gopasspw/gopass but aviau/gopass.
I'll try to provide some context on this situation.
When we started the gopass project we were looking for a name and before settling on gopass we did check for conflicting projects. At this time there were no conflicting CLI tools or packages using that name.
The getpasswd implementation howeyc/gopass was already around but since that was a Go package and those are usually referrenced by their full import path we didn't see any issue with that.
What we did miss however was the Debian ITP (intent to package) bug #781494. This was already around when we released our first version, but the actual debian package wasn't uploaded until after gopass already gained some traction.
When we did first become aware of this naming conflict (shortly after it was uploaded to Debian unstable) I did reach out to the maintainer (who is also the upstream author) to suggest different options to resolve this confusion (e.g. not having a gopass package at all in debian but two similarly named ones or joining our efforts in one project) but they did not like either of my proposals and didn't respond very favorably.
It struck me as odd that they had an ITP sitting around for a few years and only started to publish a package after we had launched our gopass project (we tried to make a bit of fuzz around it, so it's not entriely unlikely that they became aware of that).
But nonetheless as far a my knowledge of the Debian Constitution and the Debian Policy goes I think package naming is first come first serve without any feasible conflict resolution approach. As such this is another example of poor Debian policies serving as an annoyance to it's users (cf. docker vs. docker.io).
I'm not sure if there is any way to improve this situation but I don't intent to spend any more time on this. If any of our users are unhappy about the situation I'd suggest they report to their distributions bug trackers.
But as a long time Debian user it makes me very sad :/
Others
Alpine Linux
They seem to accept PRs on their GitLab instance. I'm trying to see if they accept mine and if they are willing to we could automate updates to this in the future.
Arch
The community package seems to be pretty much up to date. I haven't figured out if they also accept PRs, but if they do we could automate that, too.
Chocolatey
Pretty much up to date, too. Not sure if this could be automated.
DPorts
Seems to be related to FreeBSD ports, quite outdated. But I have not idea how these work.
FreeBSD Ports
Up to date. Thanks!
Funtoo
Somewhat outdated. Is this a rolling release distro or not?
Gentoo
Up to date, Thanks!
MacPorts
Somewhat outdated.
Manjaro
Quite up to date, nice.
OpenBSD Ports
A little bit outdated.
Solus
Pretty much up to date. The maintainer has sent PRs to us. Thanks a lot!
Termux
A little bit outdated. They seem to accept PRs but we're in the termux/unstable-packages repo and require a patch on their side.
I'd like to make the patch obsolete and get us into their regular packages repo, if possible.

Regarding the question if we can speed up adoption of new releases by e.g. sending PRs.

Alpine Linux: Accepts PRs via https://gitlab.alpinelinux.org/alpine/aports
Arch Linux: Not sure, likely not necessary.
Chocolatey: Maybe
FreeBSD: Using SVN ... :fearful:
Gentoo: Maybe, likely not necessary
Homebrew: Possible, but not necessary. They seem to be super fast already.
LiGurOS develop: Not sure. Looks like it's a Gentoo derivate.
Linuxbrew: Likely not necessary, see Homebrew
MacPorts: Seems to accept PRs, package a little bit outdated.
NixOS: Should be possible, but likely not necessary. They seem to be quite fast.
OpenBSD Ports: Using SVN ... :fearful:
Parabola Linux: Looks like a ArchLinux derivate
Scoop: Possible, likey not necessary
Solus: Not sure ...
Termux: Not sure if they accept PRs, I have https://github.com/termux/unstable-packages/pull/214 open
Void Linux: They seem to accept PRs, package slightly outdated.

Regarding debian, would it be possible to package it under a new name?
Like gopass-pw for instance?
It would greatly ease the installation process for those users (still not as ideal as gopass, but much better than the current manual steps).

@oliverpool Yes, of course. But someone else needs to own that.

Nothing to do here, I'd say. Apart from Debian this should be in good shape.


chore: Upgrade to DataFusion 43.0.0-rc1

Which issue does this PR close?

N/A
Rationale for this change

Voting has started on DataFusion 43.0.0-rc1, so we should make sure that it does not cause any regressions in Comet.
What changes are included in this PR?

How are these changes tested?

I am tempted to ignore this test for now and file a follow on issue given that we disable this feature by default.

@comphead FYI; it looks like there may be a regression in DF 43 related to sort-merge join with join filter.
I am tempted to ignore this test for now and file a follow on issue given that we disable this feature by default.

Thanks @andygrove it is FullOuterJoin filtered join and expected behavior, I'm planning to finish the FullOuterJoin in DF this week.

I ran some TPC-H benchmarks and do not see any change to performance with the DF 43 upgrade.


[OFFICIAL] Generated Issue: TypeError

Steps to reproduce:
from jikanpy import AioJikan

# Headers used in request
headers = {
    'Accept-Encoding': 'gzip',
    'Cache-Control': 'no-store',
    'Content-Type': 'application/json',
    'User-Agent': USER_AGENT,
}

page = 1
data = {"data": []}

while True:
    response = await jikan.seasons(extension="now", page=page)
    data["data"] += response["data"]

    if response["pagination"]["has_next_page"]:
        page += 1
        continue
    else:
        break

Additional Details
Jikan Parser Version: v4.0.2@4df99e0df5a4adf2b92e7c6fc663de8fd4b9c325
PHP: 8.0.19
Redis: Connected
Exception: TypeError
Code: 0
Message: Unsupported operand types: string - int
Trace: /var/www/jikan-rest/vendor/jenssegers/mongodb/src/Query/Builder.php on line 558
Request: GET /v4/seasons/now?page=v

That's a client error, but I admit that the API should tell you that you have specified the data for the page parameter in the wrong format. In this case string instead of int.

Yea you are right, the jikanpy lib is giving the wrong annotation for page parameter, should be string instead of int.

# In jikanpy-v4
# jikanpy/utils.py at line 121

if query_params != {}:
    k, v = query_params.popitem()
    url += f"?{k}=v"
    url += "".join(f"&{k}={v}" for k, v in query_params.items())

# On line `url += f"?{k}=v"`
# Should be
url += f"?{k}={v}"


Replace instances of "== True" with "is True"

See pep8 https://www.python.org/dev/peps/pep-0008/#programming-recommendations

Don't compare boolean values to True or False using == .
Yes:   if greeting:
No:    if greeting == True:
Worse: if greeting is True:

Incorrect title? The pep8 recommendation says to change it to
 if greeting:

not
if greeting is True

@sbellem are you referring to some specific part of the code?

Agree there is no need to use == with Boolean checks. Another good optimization I like is the Boolean flip. Instead of an if statement to change a bool to its opposite you can just do:  foo = !foo

Sorry for being vague. I was referring to tests.

@vrde Please let me know if you have questions.

@sbellem: Sorry for being vague. I was referring to tests.

Yes, now I understand :smile_cat:, anyway for tests we should be more relaxed with pep8 for some cases (this case is a great example). I think we can have different configuration for different directories with flake8. I'll give it a try later.

@Andygmb: Incorrect title? The pep8 recommendation says to change it to

Yup, it was a bit too generic.

@genecyber: Agree there is no need to use == with Boolean checks. Another good optimization I like is the Boolean flip. Instead of an if statement to change a bool to its opposite you can just do: foo = !foo

In this specific case (testing) using is to test against True, False, and None is the way to go, we want to be 100% sure that the result returned by a func/method is what we expect, type included.
To all: can you take a look to #63 and give me a :+1: if it's good? Thanks!


Add IPU support for HF pipelines to Whisper

Add support for HF pipelines
This PR makes it possible to use the HF pipeline for Whisper on the IPU.
Before submitting

[ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
[ ] Did you make sure to update the documentation with your changes?
[ ] Did you write any new necessary tests?

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

For automatic_speech_recognition.py can you indicate which parts are actual modifications? E.g. today in generation/utils.py we have comments like # Change: ..., perhaps # IPU Change: ... is clearer. The file seems to be mostly a copy and paste of upstream so its unclear what can be re-used and where we need IPU specific patches.


What are the A, G, B of F(2x2,1x3)?

Hi, with an example:
For this paper: https://arxiv.org/pdf/2002.00552.pdf, it describes decomposable winorgad. So when kernel is 7x7, stride = 1, we can divide the convolution procedure to kernels: 4 3x3 kernel, 2 3x1 kernel, 2 1x3 kernel, 1 1x1 kernel and then apply  winograd separately. But how to generate the A,G,B of F(2x2, 1x3), F(2x2,1x3), F(2x2, 1x1)?  I know it could be nested. For F(2x2, 1x3), F(2,3) could be nested in outer F(2,1)，so it is the problem: What's the A,G,B of F(2,1)?
Thank you.

F(2, 1) means the kernel-size is 1, a point-wise convolution, which is not really convolution at all. Point-wise convolution is mathematically equivalent to matrix multiplication. There is no fast-convolution algorithm for it.

F(2, 1) means the kernel-size is 1, a point-wise convolution, which is not really convolution at all. Point-wise convolution is mathematically equivalent to matrix multiplication. There is no fast-convolution algorithm for it.
Hope that helps!

Thanks for your Reply, dear author.  So I will consider using gemm directly for point-wise. I want to ask you for another question about the Fast covolution algorithm paper:
In the paper, the scattered weight matrix will be right prdoucted by the scattered Input matrix:
$$ M = UV$$
But for some implementations and a picture of visualizing the Winograd:

It seems that:
$$ M = VU $$.
Am I wrong about this comprehension?
Thank you sir.

Nice graphic! Did you draw it yourself?
If we write M = UV, then U must have shape K x C and V must have shape C x P (where P is the number of tiles), and M has shape K x P.  I like to write that as KC,CP->KP
If you want to write M = VU instead, then the shapes of M, V, and U are all transposed from the above shapes. I would write it as PC,CK->PK.
I don't think the diagram implies a particular tensor order.

Nice graphic! Did you draw it yourself?
If we write M = UV, then U must have shape K x C and V must have shape C x P (where P is the number of tiles), and M has shape K x P. I like to write that as KC,CP->KP
If you want to write M = VU instead, then the shapes of M, V, and U are all transposed from the above shapes. I would write it as PC,CK->PK.
I don't think the diagram implies a particular tensor order.

No, I didn't draw this diagram. It is cited from this paper: https://arxiv.org/pdf/1810.01973.pdf.
So because the element-wise multiplication, You can write M =UV or M = VU. All you need to do is to organize the data.

(AB)' = B'A'  where ' means matrix-transpose. This is a basic fact of matrix multiplication. It is a good idea to make sure you have mastered the basics of linear algebra. I learned it from "Calculus vol2" by Tom Apostol. There might be better books available now.


Refactor count implementation to improve readability

Summary or solution description
Refactored count built-in method.
Part of an overall refactoring to unify code generator logic and built-in methods generation.
How to Reproduce
https://github.com/CityOfZion/neo3-boa/blob/d23e08e2b3daa768aa7f41458b696f3ad4721874/boa3_test/test_sc/built_in_methods_test/CountListAnyType.py#L6-L16
Tests
https://github.com/CityOfZion/neo3-boa/blob/d23e08e2b3daa768aa7f41458b696f3ad4721874/boa3_test/tests/compiler_tests/test_builtin_method.py#L1692-L2166
Platform:

OS: Windows 10 x64
Python version: Python 3.7+

Task linked: CU-864eq2pze Refactor count implementation to improve readability

coverage: 91.342% (+0.05%) from 91.289% when pulling c3d99f466c9dbb79f2a83a17d0774a9fc8de7efe on CU-864eq2pze into 9f3c177aa25ca0d7d70f2749ba4dfb3477660cee on development.


MonoTorrent.ListenerStatus.PortNotFree

Hello.
sorry for my eng.
problem: can't find other clients
run the file

http://hh-soft.ru/filedownload/eyJ0b2tlbiI6IjUzYWU0ZmY1MzBhMDU0YzQ2NTAzYzg1NjliM2UxZDFhZmVkY2ZiZjM5NGRhOTNiYWFiMjljZGU4ZTBjOGE5MzQifQ==
Status MonoTorrent(VS Debugger): Engine->listener: MonoTorrent.ListenerStatus.PortNotFree  (all the time)
version monotorrent- 1.0.24
version NAT - 2.2
pc computer connection: tp-link router: use NAT
μTorrent Сlient  connects normally

If you're getting a 'port not free' it means some other program is using the port you're asking monotorrent to use.
Have you tried using a different port number in monotorrent?

I'll close this for now as it's unclear what exactly is going wrong here. Please feel free to reopen if the issue still reliably occurs and we'll dig into it!


Potential algorithmic improvements for backward pass

I removed the FP32 atomics, allowing the dQ accumulation to be done more naturally (a second backward kernel traveling in the same direction as forward). This looks easier to code, and was faster in production. Despite having more computations (9 GEMMs in FWD + BWD, the order work is dispatched during model training). Hardware these days likes higher arithmetic intensity.
Also searched a giant combinatorial space of block dimensions, and whether each operand is cached or spilled. It's more complex than GEMM with only A/B/C matrices. ~10 different operands (Q, K, V, O, log-sum-exp, dO, dV, dK, dQ). I had to battle the compiler, which wanted to use up all the registers and make occupancy plummet. A lot of competing variables to consider. Therefore, I treated the register pressure issue as a combinatorial problem.

Could you double-check my reasoning and perhaps fill in missing data for NV hardware? I want a second opinion, from someone with similar expertise regarding the FlashAttention algorithm.
https://github.com/philipturner/metal-flash-attention

This is great. Yes the backward pass is a massive pain.
Having 2 separate kernels for dK, dV and then dQ also makes sense. That's how the Triton implementation does it.
https://github.com/triton-lang/triton/blob/e0613c64b9bb872e4fa47dbce15ffa6770a1032b/python/tutorials/06-fused-attention.py#L309
The utilization mostly depends on the ratio of matmul FLOPS (e.g. tensor cores) and vector FLOPS (e.g. exponential for softmax). The higher this ratio, the harder it is to get high util.
Idk much about Apple chips, but going from A100 -> H100, the matmul FLOPS double (per streaming multiprocessor per clock cycle), but FLOPS for exponential stays the same (per SM per clock cycle).
As an example, A100 has 312 TLFOPS of FP16 matmul and 2.44 TFLOPS for exponential. The H100 has 989 TFLOPS of FP16 matmul but only 3.86 TFLOPS for exponential.
Do you know the numbers for Apple chips (I couldn't find it from a quick look).

You can issue 32 complex instructions per core-cycle on Apple chips. In comparison, you can issue 128 FMA instructions per core-cycle on Apple chips. If you issue a mixed workload where 3 of every 4 instructions is FMA, then both pipelines can be saturated simultaneously. Here are numbers across numerous architectures, for consumer GPUs. All of them appear to have 128 ALUs in the core, for the main FFMA32/IADD32 pipeline.
This table was the result of extensive research across numerous sources, from Chips and Cheese benchmarks to Intel's hardware documentation. Normalizing for different vendors reporting "what is a GPU core"? For example, AMD's dual compute unit (WGP) should be reported as two GPU cores. It is similar in memory I/O bus width to that of two CPU cores. What I consider "GPU core" has the same memory I/O bus width as an Apple Firestorm CPU or Intel AVX-2 CPU.
https://github.com/philipturner/metal-benchmarks?tab=readme-ov-file#operations-per-second

Architecture
EXP2
IMUL32
IADD32
FMUL32
FFMA32
FFMA16 (Tensor)

Apple (M1, M3)
32
32
128
128
128
128

AMD (RDNA 3)
32
16
64
128
128
256

Intel (Gen9)
TBD
TBD
TBD
64
64
128

Nvidia (Ada)
16
64
64
128
128
256

The 16-bit FFMAs are obfuscated in hardware-specific SIMD group matrix instructions. But there are dedicated ALUs on certain chips, with double the throughput of FFMA32. These happen to be Nvidia's tensor cores on consumer chips. To make an accurate analysis, I'd have to map this model of instructions per GPU core-cycle to Nvidia server hardware.

A few insights after thinking some more. The way you do dQ accumulation is very restrictive. It encourages square block sizes, to balance the bandwidth of paging/reading along two orthogonal matrix dimensions. In comparison, my approach makes no distinction between the dimensions. There is always just one where the paging overhead is amortized ("traversal"). That opens up a much larger combinatorial space for block sizes. There is a higher opportunity you will find a kernel with high performance, with my approach. Try 32-64 x 160-320.
Second, each backward kernel has a higher ratio of FFMA instructions to complex instructions, than forward. For example, all kernels have 5 softmax instructions, one of which is exponentiation. There is no reason that bottleneck should prevent these kernels from having higher ALU utilization. Only the fact that we're issuing more compute work, and assuming your 7 GEMMs have 100% utilization (which they do not, by a long shot). Perhaps the instructions that read / page stuff to memory count as "vector instructions". They could be a new bottleneck.

That's an interesting perspective. Both approaches have tradeoffs (more recompute vs more memory access).
I did think about this approach (7 GEMMs) when we rewrote for Hopper. The 5 GEMMs approached ended up at around 550 TFLOPS (for hdim 128) for the backward pass alone. For the 7 GEMMs approach to get the same speed, the GEMMs would have to be at 770 TFLOPS (since it does 7/5 = 1.4x more compute). It would be hard to get to 770 TFLOPS (the forward with 2 GEMMs can get to around 700 TFLOPS, and pure matmul with 1 GEMM can get to 750-770 TFLOPS).
For a different chip, with different ratio of compute / memory, the tradeoff could look different.

 I don't even see benchmarks for D=256. This implies that training (gradient computation) cannot be evaluated with FlashAttention. Because the accumulator consumes too many registers and the compiler spills them to RAM. People are shifting to models with larger head dimension nowaways.

The numbers for D=256 aren't there yet (as you said, registers is a big headache).
Most models use hdim 128.

There are some important models (FLUX I believe?) which have hdim 256. And on Apple hardware, with less registers, you have issues at hdim 128 that Nvidia would face at hdim 256.
At large head dimensions, you need to intentionally spill the accumulator to RAM. This shifts where the memory bottlenecks are in the kernel. Encourages highly non-square, lopsided blocks. Such as 64x128 -> 32x256. The major issue of atomic dQ, is it forces you to use square block sizes. So perhaps it reaches 77% ALU utilization, and 7/5x more computations. 7 GEMMs may be be slower or faster than the implementation currently here. But it does open up the flexibility to optimize for more variation in head dimension.

Btw I'm not sure doing dQ in a separate kernel will save registers. In the kernel with 4 GEMMs, we'd still need registers to hold the accumulator for all the 4 GEMMs. For the 5th gemm (dQ), the accumulator for dQ can use the same registers as those of QK^T and dO V^T accumulators.

Does the FlashAttention repo, as is, have a block parameter you can just tweak so it reads 256 elements in memory to support D=256? Or is the head dimension hardcoded (e.g. if I wanted a model with D=127 and the elements are laid out in memory 127 elements apart, I could not do that).

Head dimensions are hard-coded. Just like gemm tile sizes are hard-coded (usually 128 x 128 or 128 x 256 or 256 x 128).

Btw I'm not sure doing dQ in a separate kernel will save registers. In the kernel with 4 GEMMs, we'd still need registers to hold the accumulator for all the 4 GEMMs. For the 5th gemm (dQ), the accumulator for dQ can use the same registers as those of QK^T and dO V^T accumulators.

You can reduce the register allocation to nil. With this approach, both separation into two different kernels, and lopsided block sizes aligned to the traversal dimension. When the head dimension grows infinitely large.

For Hopper at least some of the block sizes need to be divisible by 64 (since the wgmma instruction needs M=64). So you can't really decrease M that much (we already use tile size 64 x 128 for hdim 128 bwd).

Head dimensions are hard-coded. Just like gemm tile sizes are hard-coded (usually 128 x 128 or 128 x 256 or 256 x 128)

I need a little more flexibility than that. From the start, I've targeted wierd and typically not benchmarked problems. Such as D=40 in Stable Diffusion v1, which was a factor of 4 slower than D=32 in Metal Performance Shaders. The code generation handles edge cases where D or sequence dimension doesn't align with block size. For example D=199 where D_block=32.

Just popping in quickly here, Flux has head dimension 128 :)
On Fri, Aug 23, 2024 at 10:49 PM Philip Turner @.***>
wrote:

Head dimensions are hard-coded. Just like gemm tile sizes are hard-coded
(usually 128 x 128 or 128 x 256 or 256 x 128)
I need a little more flexibility than that. From the start, I've targeted
wierd and typically not benchmarked problems. Such as D=40 in Stable
Diffusion v1, which was a factor of 4 slower than D=32 in Metal Performance
Shaders. The code generation handles edge cases where D or sequence
dimension doesn't align with block size. For example D=199 where D_block=32.
—
Reply to this email directly, view it on GitHub
https://github.com/Dao-AILab/flash-attention/issues/1172#issuecomment-2308141099,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABDDIIXDDYIOAR2H3E4LND3ZTANEXAVCNFSM6AAAAABNA62TDKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBYGE2DCMBZHE
.
You are receiving this because you are subscribed to this thread.Message
ID: @.***>

For Hopper at least some of the block sizes need to be divisible by 64 (since the wgmma instruction needs M=64). So you can't really decrease M that much (we already use tile size 64 x 128 for hdim 128 bwd).

On Apple, the SIMD group matrix instruction is scoped at M=8. So I can access block sizes such as 48x48 (the optimal one for Apple, which is strangely not a power of 2). And 32x80, is the best for attention on M1 architecture. Also sometimes played around with the D block dimension being 24.

Just popping in quickly here, Flux has head dimension 128 :)

Must be another model. But it's important to have code that can work with large head dimensions. @liuliu perhaps can say which model he's targeting with D=256.

Would this test even compile with the FlashAttention repo? Forget executing fast, would I even get correct results running this test suite on a CUDA device?
https://github.com/philipturner/metal-flash-attention/blob/749e742ac27d01a63f88b9e25a566e70dfac5632/Tests/FlashAttentionTests/Attention/SquareAttentionTest.swift#L5-L26

It would run and get correct results. For hdim not divisible by 8 we explicitly pad (which would make it slower). This is mostly so that we can use TMA (which requires alignment by 16 bytes). If one really wants, one can forgo TMA and use standard copying, but that's more code to write, so we don't do that.
For hdim not multiples of 32 or 64, we implicitly pad. There are kernels for hdim 64, 96, 128, 192, 256. Any hdim not in those 5 will get rounded.

This is what Apple does as well. The SIMD async copy instruction can pad matrix dimensions up to multiples of 8. Don’t forgo TMA and use standard copying. The hardware support for unaligned matrix edges (e.g. D=127) is the proper way to code this.

There are kernels for hdim 64, 96, 128, 192, 256.

Does this mean it’s technically possible to get benchmarks for A100/H100 training performance with D=256?

TMA requires addresses to be multiples of 16 bytes, so if you have D=127 TMA won't work. This is standard in GEMM as well, where e.g. Nvidia recommends that dimensions be multiples of 16 bytes (https://docs.nvidia.com/deeplearning/performance/dl-performance-matrix-multiplication/index.html#requirements-tc).
There's limited time for open-source work, and we've focused more on common dimensions that most models use.
Yes FA2 hdim 256 is supported. FA3 has fwd for hdim 256 but bwd for hdim 256 is not out yet.
Ultimately the choice of 5 GEMM or 7 GEMM bwd is empirical: the right thing is to implement both and pick what's faster. For hdim128 on H100 I'm convinced that 5 GEMM is faster (by the reasoning above where the 7 GEMM approach would have to get to 770 TFLOPS, which is already not easy for 1 GEMM). For different hdim, different chip, I could see 7 GEMM being faster.
Note that the 7 GEMM approach has the advantage that it gives deterministic results. That might be one reason the Triton implementation went with it.

This reasoning makes sense. There are not a lot of models out there with hdim 256. On Apple hardware, I had less register memory, and FP32 atomics were not supported in hardware. The 5 GEMM approach would be extremely complex to implement, and unnecessarily constrained the block sizes to be squares. I would love to see benchmarks of A100 / H100 doing FlashAttention with hdim 256, just to see how well my implementation compares to yours.
I designed my codebase to use code generation and benchmarking across the entire spectrum of problem sizes. It requires little effort for me to test hdim 256, 257, 384, 385, etc. However, it seems the main repo hard-codes the head dimensions, and changing hdim requires a lot of effort. This is a common practice, for example PyTorch FlexAttention only supports matrix dimensions divisible by 128. It is much easier to write kernels restricted to integer powers of 2 (they can be handwritten). The odd / misaligned matrices are more challenging, that's why I needed code generation.
I'll have to leave this investigation unanswered for the time being, move on to other things. Thanks for the insights though.

There are some important models (FLUX I believe?) which have hdim 256. And on Apple hardware, with less registers, you have issues at hdim 128 that Nvidia would face at hdim 256.

Just to reference this, I meant the AuraFlow model which uses hdim 256 (and trained on H100). FLUX.1 uses hdim 128.

I changed the numbers to say 0% for the missing data points, because D=256 gradient cannot even be evaluated in the main repository. I wonder how the AuraFlow model was trained.

No FA2 has bwd for D=256.

Would you be able to execute backward for D=256 on H100 or A100? I don't have access to one of those GPUs. If it's only for A100, I'm okay with that data point.

Yes it runs on both A100 and H100 (not using new H100 features). Util on A100 is around the same as hdim64. Util on H100 is not great (obv), like 250-270 TFLOPS.

Can I have something specific and quantitative?

 Would need to find an A100 :D
I mostly work w H100.

I’ll take whatever I can get. I want to examine how Flash2 scales with increasing head dimension on the H100. That’s sufficient to get the conclusion I’m after.

This thing, right here, but on the H100 + Flash2 instead of the M1 Max + MFA. Perhaps you'll need to batch a bunch of attentions in parallel to fully saturate the parallelism of H100.
  // Benchmark performance.
  var maxGINSTRS: Int = .zero
  for _ in 0..<5 {
    let dispatchCount: Int = 5
    let latencySeconds = executeCommandBuffer(dispatchCount: dispatchCount)

    // Determine the amount of work done.
    //
    // WARNING: Change this code to match the kernel you're profiling.
    var operations: Int
    switch benchmarkedKernel {
    case .forward:
      operations = 2 * headDimension + 5
    case .backwardQuery:
      operations = 3 * headDimension + 5
    case .backwardKeyValue:
      operations = 4 * headDimension + 5
    }
    operations *= (sequenceDimension * sequenceDimension)
    operations *= dispatchCount

    // Divide the work by the latency, resulting in throughput.
    let instrs = Double(operations) / Double(latencySeconds)
    let ginstrs = Int(instrs / 1e9)

    // Accumulate the sample from this trial.
    maxGINSTRS = max(maxGINSTRS, ginstrs)
  }
  return maxGINSTRS

Then, automate a test over 38 points in the spectrum of D dimensions.
    //    var D_array: [Int] = []
    //
    //    var D_cursor = 0
    //    while D_cursor < 96 {
    //      D_cursor += 4
    //      D_array.append(D_cursor)
    //    }
    //    while D_cursor < 160 {
    //      D_cursor += 8
    //      D_array.append(D_cursor)
    //    }
    //    while D_cursor < 256 {
    //      D_cursor += 16
    //      D_array.append(D_cursor)
    //    }
    //    while D_cursor < 384 {
    //      D_cursor += 32
    //      D_array.append(D_cursor)
    //    }

And finally, a beautiful graph like this. Modified to suit the differences between the MFA and Flash2 repos.


Adjust/fix API for working with vector tile features

This pull request suggests three changes:

Fix tile load events for ol.source.VectorTile: Before #6779, when registering for tile load events on ol.source.VectorTile, load events were fired for ol.VectorTile instances. Now they are fired with ol.VectorImageTile instances, which is problematic because these tiles neither match the tileGrid that the source is configured with, nor the tiles that are passed to the tileLoadFunction of the source.
Make ol.VectorTile#getFeatures and #getProjection exportable.
Remove ol.VectorImageTile from the API.

Background:
For applications that need to work with vector tile features/geometries, it is currently impossible to do so. By monitoring tile loads, and having access to the tile grid and each tile's features, it should be possible to transform feature geometries from tile pixels to view coordinates. A possible workflow would be the following:
var tiles = {};
vectorTileSource.on('tileloadend', function(e) {
  tiles[e.tile.getTileCoord()] = e.tile;
});
// Now we know the loaded tiles, let's get the geometries inside the viewport
vectorTileSource.getTileGrid().forEachTileCoord(view.calculateExtent(), Math.round(view.getZoom()), function(tileCoord) {
  var tile = tiles[tileCoord];
  var features = tile.getFeatures();
  var projection = tile.getProjection();
  if (projection.getUnits() == 'tile-pixels') {
    // Transform feature geometry coordinates
    // Do something with geometries
  }
});

Updated description to better explain what this pull request does.

I want to get features from vectortile and get the features's coordinate。I try this way,but failed.
Can you give me some example?

Sorry, I don't have a complete example. If you share your attempt in a codepen, I can help you make it work though.


 Make sure you are using the latest version; type  youtube-dl -U  to update. Be sure to call youtube-dl with the --verbose flag and include its complete output.',)
[debug] Invoking downloader on 'https://r4---sn-5hnedn7s.googlevideo.com/videoplayback?id=aa53f1ead9802253&itag=140&source=youtube&requiressl=yes&mm=31&pl=18&mn=sn-5hnedn7s&mv=m&ms=au&nh=IgpwcjAyLmFtczA0KgkxMjcuMC4wLjE&ratebypass=yes&mime=audio/mp4&gir=yes&clen=3340767&lmt=1431930633820517&dur=210.303&mt=1436616180&key=dg_yt0&upn=H-RYIjfvPnQ&fexp=901453,901816,9406990,9408142,9408420,9408710,9416126&signature=97155D5B090A9E384D94F10284C51DF53C82BE11.1047172D758CB6B370B7FE701C2B63D10EF353D5&sver=3&ip=<IP_ADDRESS>&ipbits=0&expire=1436637895&sparams=ip,ipbits,expire,id,itag,source,requiressl,mm,pl,mn,mv,ms,nh,ratebypass,mime,gir,clen,lmt,dur'
[download] Madcon - Don't Worry (Official Lyric Video) ft. Ray Dalton-qlPx6tmAIlM.m4a has already been downloaded
[download] 100% of 3.18MiB
[ffmpeg] Correcting container in "Madcon - Don't Worry (Official Lyric Video) ft. Ray Dalton-qlPx6tmAIlM.m4a"
[debug] ffmpeg command line: ffmpeg -y -i 'Madcon - Don'"'"'t Worry (Official Lyric Video) ft. Ray Dalton-qlPx6tmAIlM.m4a' -c copy -f mp4 'Madcon - Don'"'"'t Worry (Official Lyric Video) ft. Ray Dalton-qlPx6tmAIlM.temp.m4a'
youtube-dl --version
Output:
2015.07.07

Duplicate of #5118.


Pre-Publish Checklist: Use WASM AMP Validator

Context
https://github.com/ampproject/amphtml/issues/36067#issuecomment-919532568
Summary
The current way we're validating stories in the checklist of the editor is getting deprecated, so we need to update to use https://cdn.ampproject.org/v0/validator_wasm.js
Relevant Technical Choices
Using the _wasm version of the validator script requires us to use WebAssembly to instantiate the validator and then validate the string.
The problem I'm running into is with the script getting loaded ahead of time I'm unable to figure out how to make the validator from window. into an ArrayBuffer and get something readable for WebAssembly.
The new script gives us window.amp.validator like the previous one, but it won't work without web assembly and there's no promise as far as I can tell to access there. the new script also gives us window.loadValidatorWasm which seems like what we want because wasm - that's a function, waiting on it will give you all the same options that the amp.validator does but it has a promise there with it.

Pretty much, It's catching on WebAssembly and throwing an error in the try/catch.
I've kept console logs here to try and make debugging easier. I'm sure i've been staring at this too long and that someone with more knowledge about WebAssembly and the validator can probably figure this out faster.
I also have it as a codepen with the same console errors if you don't want to run the branch: https://codepen.io/brittanyIRL/pen/MWoQOrO?editors=1011

Update
I was just doing this wrong. Thought we needed to use WebAseembly to gain access to validating, but really we just needed to import and initialize. Thanks (again) @swissspidy.
To-do

[x] One validator script works need to swap out what we're looking for: https://github.com/ampproject/amphtml/blob/cfb102aafc1543788b77f3ef9bd138c753e13097/validator/validator.proto#L853

User-facing changes

Testing Instructions

[ ] This is a non-user-facing change and requires no QA

This PR can be tested by following these steps:

Reviews
Does this PR have a security-related impact?

Does this PR change what data or activity we track or use?

Does this PR have a legal-related impact?

Checklist

[x] This PR addresses an existing issue and I have linked this PR to it in ZenHub
[x] I have tested this code to the best of my abilities
[ ] I have verified accessibility to the best of my abilities (docs)
[ ] I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
[ ] This code is covered by automated tests (unit, integration, and/or e2e) to verify it works as intended (docs)
[ ] I have added documentation where necessary
[x] I have added a matching Type: XYZ label to the PR

Fixes #8477

@BrittanyIRL I think all you were missing is a call to amp.validator.init().
With that, the validator runs just fine.
The weird thing is I'm getting the following STYLESHEET_AND_INLINE_STYLE_TOO_LONG error for a blank story when I should be getting MISSING_URL because of a missing story poster image:
{
    "severity": "ERROR",
    "code": "STYLESHEET_AND_INLINE_STYLE_TOO_LONG",
    "line": 6,
    "col": 2864,
    "specUrl": "https://amp.dev/documentation/components/amp-story",
    "paramsList": [
        "poster-portrait-src",
        "amp-story"
    ],
    "params": [
        "poster-portrait-src",
        "amp-story"
    ]
}

No idea why that happens yet. Looking into it a bit.

@googlebot I consent

I think all you were missing is a call to amp.validator.init().
With that, the validator runs just fine.

😭  but also 🎉

The weird thing is I'm getting the following STYLESHEET_AND_INLINE_STYLE_TOO_LONG error for a blank story when I should be getting MISSING_URL because of a missing story poster image

That's odd, i wonder why! i'll poke around a bit too later today now that the validator's actually working.

Looking at the list of errors there's no longer any note about the publisher logo and the missing_url - but in the error returned for the inline style too long it specifies the poster portrait src which makes me think it's addressing the same thing.
Adding in a publisher logo on a blank story resolves the inline style too long.

Looking at the list of errors there's no longer any note about the publisher logo and the missing_url - but in the error returned for the inline style too long it specifies the poster portrait src which makes me think it's addressing the same thing.

Yes it  does reference the poster attribute, but the error code is clearly wrong and doesn't match what you get on validator.amp.dev, which is why I wanted to look into this.
This needs further clarification before merging ⚠

Adding in a publisher logo on a blank story resolves the inline style too long.

I assume you meant a poster image?

Opened https://github.com/ampproject/amphtml/issues/36112 for looking into the error code issue

Opened ampproject/amphtml#36112 for looking into the error code issue

Bug has been confirmed; fix is being worked on

The validator error code bug has now been fixed!


bin/ycsb gives a TypeError when there is an error from Maven

Traceback (most recent call last):
File "bin/ycsb", line 261, in
sys.exit(main())
File "bin/ycsb", line 234, in main
maven_says = get_classpath_from_maven(project)
File "bin/ycsb", line 180, in get_classpath_from_maven
+ err.output)
TypeError: cannot concatenate 'str' and 'NoneType' objects

reported in #700


x/text/unicode/norm: TestLinking flaky/failing

The test for golang.org/x/text/unicode/norm appears to be flaky on linux-s390x-ibm and persistently failing on openbsd-arm.
Possibly related to #17538.
s390x:
--- FAIL: TestLinking (0.56s)
    normalize_test.go:937: tables appear not to be dropped: 2190701 - 2185419 = 5282
FAIL
FAIL	golang.org/x/text/unicode/norm	2.312s

openbsd/arm:
--- FAIL: TestLinking (8.20s)
    normalize_test.go:937: tables appear not to be dropped: 2085598 - 2081590 = 4008
FAIL
FAIL	golang.org/x/text/unicode/norm	29.639s

Looks like the openbsd-arm builder is somehow happy again, and the s390x builder appears to be gone.


Use the Repository's "boss status" and "quirks" fields

This more closely aligns what we consider a "boss module" with what the repo considers one; using the presence of an ignore list results in things like Four Card Monte and Encryption Bingo both being considered bosses when they don't (or in EB's case, no longer) actually act like one.
Default announcements are now for boss modules + pseudo needies + any module that needs immediate attention. An additional automatic profile to disable pseudo needies has been added. Obviously more can be done with quirks later, but this is a good start.

This looks good to me. Thanks for helping out! Merging...


Adjust definition of RepetitionTime in section 8.3.3 and add two new fields in section 8.3.2

PROPOSAL: Adjust the definition of RepetitionTime in section 8.3.3 Task (including resting state) imaging data and add two new fields to section 8.3.2 Anatomy imaging data.
Justification: RepetitionTime is currently defined very specifically as relating to functional imaging data. However there are structural scans that collect multiple volumes during an acquisition. Here we adjust the definition of RepetitionTime in section 8.3.3 and add RepetitionTimeExcitation and RepetitionTimeInversion as two additional terms for structural acquisitions that include multiple contrasts.
See Proposal_RepetitionTime.md for details.

That's a +1 from me! Thanks for putting this together.
Best,
Chris
On Mon, Sep 3, 2018 at 10:35 AM Kirstie Whitaker<EMAIL_ADDRESS>wrote:

PROPOSAL: Adjust the definition of RepetitionTime in section 8.3.3 Task
(including resting state) imaging data
https://docs.google.com/document/d/1HFUkAEE-pB-angVcYe6pf_-fVf4sCpOHKesUvfb8Grc/edit#bookmark=id.jm7qgqg5x2on
and add two new fields to section 8.3.2 Anatomy imaging data
https://docs.google.com/document/d/1HFUkAEE-pB-angVcYe6pf_-fVf4sCpOHKesUvfb8Grc/edit#bookmark=id.3pszfzgi4dpj
.
Justification: RepetitionTime is currently defined very specifically as
relating to functional imaging data. However there are structural scans
that collect multiple volumes during an acquisition. Here we adjust the
definition of RepetitionTime in section 8.3.3 and add
RepetitionTimeExcitation and RepetitionTimeInversion as two additional
terms for structural acquisitions that include multiple contrasts.
Add to 8.3.2 Anatomy imaging data
Some meta information about the acquisition MAY be provided in an
additional JSON file. See Common MR metadata fields for a list of terms and
their definitions. There are also some OPTIONAL JSON fields specific to
anatomical scans:

RepetitionTimeExcitation: The time in seconds between successive
excitation pulses that excite the same tissue. As with
RepetitionTimeInversion this corresponds to DICOM Tag 0018, 0080
http://dicomlookup.com/lookup.asp?sw=Tnumber&q=(0018,0080): “the
period of time … between the beginning of a pulse sequence and the
beginning of the succeeding (essentially identical) pulse sequence”. Note
that although this would typically be called RepetitionTime please use
RepetitionTimeExcitation for structural scans with multiple
excitations as RepetitionTime is already defined as the amount of time
that it takes to acquire a single volume in section 8.3.3 and to
distinguish it from RepetitionTimeInversion.
RepetitionTimeInversion: The time in seconds between successive
inversion pulses in an inversion recovery (IR) sequence, such as MP(2)RAGE.
As with RepetitionTimeExcitation this corresponds to DICOM Tag 0018,
0080 http://dicomlookup.com/lookup.asp?sw=Tnumber&q=(0018,0080):
“the period of time … between the beginning of a pulse sequence and the
beginning of the succeeding (essentially identical) pulse sequence”. Note
that although this would typically be called RepetitionTime please use
RepetitionTimeInversion for structural scans with successive inversion
pulses as RepetitionTime is already defined as the amount of time that
it takes to acquire a single volume in section 8.3.3 and to distinguish it
from RepetitionTimeExcitation.

Change in 8.3.3 Task (including resting state) imaging data
Some meta information about the acquisition MUST be provided in an
additional JSON file.
Required fields:

RepetitionTime: The time in seconds between the beginning of an
acquisition of one volume and the beginning of acquisition of the volume
following it (TR). When used in the context of functional acquisitions this
parameter best corresponds to DICOM Tag 0020,0110
http://dicomlookup.com/lookup.asp?sw=Tnumber&q=(0020,0110): the
“time delta between images in a dynamic of functional set of images” but
may be found in DICOM Tag 0018, 0080
http://dicomlookup.com/lookup.asp?sw=Tnumber&q=(0018,0080): “the
period of time in msec between the beginning of a pulse sequence and the
beginning of the succeeding (essentially identical) pulse sequence”.
(Remember that DICOM measures must be converted from milliseconds to
seconds to fit the BIDS specification.) Note that the definition includes
time between scans when no data has been acquired in the case of sparse
acquisition schemes. This value needs to be consistent with the '
pixdim[4]' field (after accounting for units stored in 'xyzt_units'
field) in the NIfTI header. This field is mutually exclusive with
VolumeTiming.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/INCF/BEP001/issues/4, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAOkp6DtdN5IXq3q8eaAAhPnPdLy5-vJks5uXWhGgaJpZM4WX6Zw
.

Thanks @chrisfilo! I've adjusted the issue and linked to a file that make super clear what I'm suggesting. The content is the same as the wording in the issue when you read it though 😸.

There was also a +1 from @effigies on the mailing list too and no other comments....so this looks ready to go!

Mathieu Boudreau (@mathieuboudreau) made the following comment in the Google doc:

In MPRAGE and IR, the inversion pulse acts as a preparation pulse for the magnetization prior to the imaging readout. This pattern repeats itself in several other quantitative MRI techniques (e.g. quantitative magnatization transter, where the preparation pulse here is instead an off-resonance pulse of a specific shape and length). Would it not be better to have a RepetitionTimePreparationPulse-type general parameter instead of having several one for each types of preparation pulses (e.g. RepetitionTimeExcitation, RepetitionTimeInversion, RepetitionTimeMTPulse, etc)? Another example is Saturation Recovery T1 mapping, where the preparation pulse is a 90 degree pulse, instead of a 180. In MRI literature, these are all simply referred to as "repetition time", so it's unclear to me why we need several names to distinguish each of these.

I think this is worth to think about.

Maybe "PulseSequenceRepetitionTime" would be a better option than splitting them up?

Maybe "PulseSequenceRepetitionTime" would be another option?

RepetitionTimePulseSequence? :P

The current text says:

When used in the context of functional acquisitions this parameter best corresponds to DICOM Tag 0020,0110: the "time delta between images in a dynamic of functional set of images" but may be found in DICOM Tag 0018, 0080: "the period of time in msec between the beginning of a pulse sequence and the beginning of the succeeding (essentially identical) pulse sequence".

The DICOM tag 0018, 0080 is "wrong" for segmented EPIs where multiple shots are made per volume. I would leave that out or maybe even say it does not correspond. What do you hink?

I think specifying that tags 0020,0110 and 008,0080 only match for the single-shot EPI case is worth mentioning if it's kept. Otherwise yeah, remove the mention to 008,0080, or explain how it differs since they can sometimes (but not always) match.

Hi,
as I commented on Agah's spreadsheet, if a student on our MSc course wrote "Inversion Repetition Time" or similar on an exam, I would fail them. You will not find that as a term in the physics literature anywhere, and it conflates two very separate properties of a sequence.
In my opinion, the usage of "TR" to refer to the time between imaging/readout pulses is well established. That's the convention in the original MP2RAGE paper (https://www.sciencedirect.com/science/article/pii/S1053811909010738, equation A1.1), the Deichmann MPRAGE paper (DOI: 10.1006/nimg.2000.0601) and every other paper I can think of.
To process an MP2RAGE dataset (which is where I think the need for these terms has arisen), you need to know TR, the number of readouts in one segment (ETL), the two Inversion Times (TI), and also the delay time (TD) after the end of the second readout train. This then defines the "other" repetition time, which is equal to TI2+ETL*TR+TD
In the MP2RAGE paper the "other" repetition time is referred to as "MP2RAGETR", which is a mouthful. In the Deichmann MPRAGE paper, this quantity is not named explicitly.
In QUIT, I refer to this quantity as the "Segment TR", shortened to SegTR in the input files (https://quit.readthedocs.io/en/sphinx/Docs/Relaxometry.html#qimp2rage). This is a terminology picked up from our old Agilent scanner, but in my opinion it is the correct one, as an MPRAGE type sequence is inherently a segmented sequence.

Hey Tobias,

as I commented on Agah's spreadsheet, if a student on our MSc course wrote "Inversion Repetition Time" or similar on an exam, I would fail them. You will not find that as a term in the physics literature anywhere, and it conflates two very separate properties of a sequence.

Thanks for your feedback. It is helpful to know that the BIDS community would fail your MSc course. It shows that we need people like you to align BIDS with the MR physics literature!
Can you tell me where I can find Agah's spreadsheet? I am afraid I have not seen it.
So I think you proposed three alternatives here:

To use a set of other parameters to calculate the MP2RAGETR. Off the top of my that means we would now also have to include Time Delay and echo train length into our spec and let developers include this calculation. To me, this seems a bit less user-friendly and a bit more error-prone than other alternatives.
Use the moutfhul of "MP2RAGETR". This would be very specific to MP2RAGE sequences though.
Your preference seems to be to use the term RepetitionTimeSegment or something similar. This seems very close to the proposal of using RepetitionTimePulseSequence. Can you explain why you like RepetitionTimeSegment more? Do you have a pointer to MR physics literature where this term is used?

Hello,
So first off - apologies for my strong wording on Friday. After I had some time to think about it, InversionRepetitionTime is not actually such a bad name - as it is the amount of time it takes to repeat the inversion pulse (I still think the likelihood is that one of our students would have mixed TI and TR if they ever wrote that though). However, I think my initial reaction shows you the kind of problem you are going to face a lot trying to write this standard - these terms are not even standardised among physicists! You also have the perennial MR problem of how you would translate the written/spoken term into a symbol for the equations, especially given that MR is the only topic (to my knowledge) that allows multi-letter combinations to represent one term. Repetition Time is TR, Inversion Time is TI, so Inversion Repetition Time would be???? TIR????
In answer to your points:

You will have to add one of either Time Delay or Echo Train Length anyway. To uniquely determine the timing of the MP2RAGE sequence, you need to know (using my terminology for a moment) TI1, TI2, ETL, TR & TD (and also if the sequence is centric or linear). If you know SegTR, then you can calculate one of those terms from the others. On the scanners I use, SegTR is the parameter on the console, not TD, so I ask users for that and then calculate TD.

Note that Echo Train Length is a term that is most commonly for fast-spin echo imaging where one initial 90 degree flip is refocussed into multiple echoes, whereas here we generate fresh gradient echoes after each readout pulse. Segment Length might be a better term, but I've not seen that used in the literature.
2/3. If you want to generalise it, you could go for PreparationRepetitionTime. After all, MP-RAGE does stand for Magnetisation Prepared Rapid Acqusition of Gradient Echoes, and the Inversion prep in what everyone calls MP2RAGE is a special case of that (Although the authors themselves broke this by coming up with the SA2RAGE sequence and not renaming MP2RAGE to IR2RAGE). Hmmm, I think I like PreparationRepetitionTime. Could go down as TP in equations.
Final note - I'd prefer to have the terms written "XRepetitionTime" rather than "RepetitionTimeX", I think it reads better in English.

Hi @spinicist! Thank you for commenting - I'm sorry for the slow reply.
I'm not 100% following exactly what you're proposing to add to the standard. There's a proposal here: https://github.com/INCF/BEP001/blob/repetitiontime/Proposal_RepetitionTime.md. Maybe you could submit a pull request to that branch with the additional wording that you're recommending.
One note: the RepetitionTimeX vs XRepetitionTime discussion was one that had already been had extensively before you joined the project. We voted on RepetitionTimeX so that the metadata was listed in alphabetical order to help human readers understand the distinctions. If you feel really strongly about re-opening that decision you're of course welcome to, but I just wanted you to know that there has already been a vote on that point, and as it doesn't affect the usage of BIDS metadata (only the human readable aspect) then it might not be an important conversation to re-have.

Hello,
My main post was on the Google group, the comment here was an extension of that.
I'm clearly late to the party here, so I don't really want to get in your way. However, I will note that if you wanted all the timing parameters in the meta-data to show up next to each other in alphabetical order, then Time should have been the first term (i.e. both RepetitionTimeX and XRepetitionTime are suboptimal), as otherwise EchoTime will be at the other end of the list. I am fairly certain this is why the TR, TE, T1, etc. convention was adopted for equations, to emphasise that these are all times.
For this particular proposal, I'll note that:

In the case of MP2RAGE, adopting a separate RepetitionTimeExcitation and leaving RepetitionTime as the "volume" time isn't very helpful, because the two inversion images are acquired interleaved. Hence there is no "volume" time - it's just the total scan time.
For any 2D scans (e.g. diffusion) the RepetitionTime is also the "volume" time. The distinction here is not structural versus functional, it's 2D versus 3D. 2D structural/anatomy scans are rare in research but they are still used widely for radiography.

Thanks @spinicist!
We can't change (for example) EchoTime or RepetitionTime as they are already part of the BIDS standard. Changing these names for BIDS 2.0 can be noted in the BIDS 2.0 document.
I'm still not clear on what you're recommending instead of adding RepetitionTimeX (where X is Inversion and Excitation)?
I'm also happy to adjust the wording if you can propose clearer phrasing.

I am recommending not adding RepetitionTimeExcitation. I can't think of a situation where it would be useful. If someone can give an example, I'll change my mind.
For RepetitionTimeInversion, I think RepetititionTimePreparation is more general. As noted above, in the SA2RAGE sequence the prep module is saturation, not inversion. There are also T2-prepared and diffusion-prepared versions of MPRAGE in existence (but not in widespread usage). RepetitionTimePreparation makes sense for all of those, whereas RepetitionTimeInversion would not.

Hey Tobias,
Thanks for your feedback! This issue goes back to a long and heated debate
in 2017. Let me summarize it once more:
The reason we have to add RepetitionTimeExcitation is that in sequences
like MP2RAGE we have to distinguish between the time between
preparation/inversion pulses (e.g., "TR_MP2RAGE" in Marques et al.) and the
time between excitations pulses in the GRE readout blocks. (e.g., "TR of
the GRE module" in Marques et al. )
If history would have played out differently, and RepetitionTime" in the
main BIDS specification was defined as DICOM tag (0018, 0080), 1C "The time
in ms between two successive excitations of the same volume. ", we could
have used RepetitionTime, like you reccomend here.
However, the definition of RepetitionTime in the BIDS spec 1.0 is as
follows:

The time in seconds between the beginning of an acquisition of one volume
and the beginning of acquisition of the volume following it (TR).

This is, thus, the time it takes to acquire an entire volume, which in the
case of 2D acquisitions is the same as the time between succesive
excitations, but not in 3D sequences, like most structural acquisitions.
We decided we cannot change the BIDS 1.0 specification post-hoc and to be
fully unambiguous we have to add another tag, namely
RepetitionTimeExcitation. Another solution would be to have different
definitions for RepetitionTime for functional and structural scans, but we
think that would be even more confusing.
Please let me know if you have any other questions about this, or maybe you
another way of solving the conundrum :).
I like your idea of using RepetititionTimePreparation. Let's discuss it
next meeting.
Cheers,
Gilles
On Tue, Nov 20, 2018 at 10:08 AM Tobias Wood<EMAIL_ADDRESS>wrote:

I am recommending not adding RepetitionTimeExcitation. I can't think of
a situation where it would be useful. If someone can give an example, I'll
change my mind.
For RepetitionTimeInversion, I think RepetititionTimePreparation is more
general. As noted above, in the SA2RAGE sequence the prep module is
saturation, not inversion. There are also T2-prepared and
diffusion-prepared versions of MPRAGE in existence (but not in widespread
usage). RepetitionTimePreparation makes sense for all of those, whereas
RepetitionTimeInversion would not.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/bids-standard/bep001/issues/4#issuecomment-440197287,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABD4zkcMUlTccrv3mIOs0qBZEm6VtQz-ks5uw8aUgaJpZM4WX6Zw
.

--
Gilles de Hollander
Postdoctoral Researcher
Department of Experimental and Applied Psychology, http://www.vupsy.nl/
https://webmail.login.vu.nl/OWA/redir.aspx?C=MeXKTnHrb7rqNpc5d9_z4HY5RYvDrp4dMHZjA7VgV9-renkmXN7UCA..&URL=http%3A%2F%2Fwww.vupsy.nl%2F
Faculty of Behavioural and Movement Sciences
Van der Boechorststraat 1, 1081 BT  Amsterdam | Room 1B75
website http://www.gillesdehollander.nl/ | github
https://github.com/Gilles86/

Hi @Gilles86 - please see my response above yours.
Currently the proposal is to add both RepetitionTimeExcitation and RepetitionTimeInversion (or my preference RepetitionTimePreparation), which when added to RepetitionTime will result in 3 terms, one of which will be entirely superfluous. This an opportunity for error and confusion, so I don't think all 3 entries should exist.
To use your own example, in an MP2RAGE sequence the BIDS 1.0 definition of RepetitionTime is entirely meaningless. What value would you set this to for an MP2RAGE image?

Hi @spinicist. I'm very confident that @Gilles86 had read your reply. Please make sure to stay as collegiate as possible in this discussion which, as Gilles said, has been going on for more than a year with some very passionate conversations causing frayed nerves all around.
RepetitionTime is not a required field for structural images, so it would not be set for a structural image that needed RepetitionTimeExcitation and RepetitionTimePreparation.
I can't think of a way of rephrasing the current definition of RepetitionTime that's in the BIDS that doesn't introduce more confusion for the large proportion of users who will use RepetitionTime to capture the time between volumes during a standard EPI functional MRI scan.

If only one of RepetitionTime and RepetitionTimeExcitation is present then I have no further objections.

Coolio - I'll leave myself an action here to make sure that that is clear in the proposal (I'm just figuring some bug out at the moment 🐛)

[ ] Action: make sure it is clear that RepetitionTime is not necessary for structural scans that have other RepetitionTimeX metadata.

How about RepetitionTime + RepititionTimePulseSequence (to be explicit that it's the MR physicist's TR)?
I think it's as general as ReptitionTimePreparation, but a bit less confusing/more clear (I know, I'm the one who first recommended that name ??)
Télécharger Outlook pour Androidhttps://aka.ms/ghei36

From: Tobias Wood<EMAIL_ADDRESS>Sent: Tuesday, November 20, 2018 8:13:28 AM
To: bids-standard/bep001
Cc: Mathieu Boudreau; Mention
Subject: Re: [bids-standard/bep001] Adjust definition of RepetitionTime in section 8.3.3 and add two new fields in section 8.3.2 (#4)
If only one of RepetitionTime and RepetitionTimeExcitation is present then I have no further objections.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/bids-standard/bep001/issues/4#issuecomment-440244590, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABWu5SFnRLmzViMOUwnBfdl3hwgzWorwks5uw-rggaJpZM4WX6Zw.

@mathieuboudreau - this is the same question as I had for @spinicist: can you suggest how you'd redefined RepetitionTime from the definition that's currently in the BIDS specification to make it clearer?
How would you define RepetitionTimePreparation?
For reference, my attempt at defining the updates is at: https://github.com/bids-standard/bep001/blob/repetitiontime/Proposal_RepetitionTime.md

Oh sorry for that. I'm only on my phone at the moment, but the definition on MR-Tip looks to be close to what I believe it is, and more general than the time between succesive excitation pulse: https://www.mr-tip.com/serv1.php?type=db1&dbs=Repetition Time
What do you think @spinicist?
Télécharger Outlook pour Androidhttps://aka.ms/ghei36

From: Kirstie Whitaker<EMAIL_ADDRESS>Sent: Tuesday, November 20, 2018 8:25:19 AM
To: bids-standard/bep001
Cc: Mathieu Boudreau; Mention
Subject: Re: [bids-standard/bep001] Adjust definition of RepetitionTime in section 8.3.3 and add two new fields in section 8.3.2 (#4)
@mathieuboudreauhttps://github.com/mathieuboudreau - this is the same question as I had for @spinicisthttps://github.com/spinicist: can you suggest how you'd redefined RepetitionTime from the definition that's currently in the BIDS specification to make it clearer?
How would you define RepetitionTimePreparation?
For reference, my attempt at defining the updates is at: https://github.com/bids-standard/bep001/blob/repetitiontime/Proposal_RepetitionTime.md
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/bids-standard/bep001/issues/4#issuecomment-440247531, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABWu5aG9wIW2DkhdVlV1XZmkfq9DrGDVks5uw-2ngaJpZM4WX6Zw.

@KirstieJane / @mathieuboudreau I think I am missing the point of the question, but if you used your definition of RepetitionTimeExcitation as the definition of RepetitionTime then it resolves everything. The time between successive excitations of the same tissue is the volume time in a 2D sequence.
If you swap all instances Inversion for Preparation in the current definition of RepetitionTimeInversion, and then say "For example, in an MP2RAGE sequence this would be the time between successive inversion pulses" I think it is clear.

My feeling is that that is too big a change to the Task (including resting state) imaging data section of the specification for the majority of BIDS users.
I don't have a problem with Preparation instead of Inversion - happy to discuss in the meeting on Wednesday 28.
I have to head into another meeting. If either of you have time to submit a pull request to update my proposal it would be really helpful to have something concrete to discuss.

@KirstieJane https://github.com/KirstieJane / @mathieuboudreau
https://github.com/mathieuboudreau I think I am missing the point of
the question, but if you used your definition of RepetitionTimeExcitation as
the definition of RepetitionTime then it resolves everything. The time
between successive excitations of the same tissue is the volume time in a
2D sequence.

We can not do this, because we cannot change the current BIDS specification
of RepetitionTime post-hoc.
On Tue, Nov 20, 2018 at 1:05 PM Tobias Wood<EMAIL_ADDRESS>wrote:

@KirstieJane https://github.com/KirstieJane / @mathieuboudreau
https://github.com/mathieuboudreau I think I am missing the point of
the question, but if you used your definition of RepetitionTimeExcitation
as the definition of RepetitionTime then it resolves everything. The time
between successive excitations of the same tissue is the volume time in a
2D sequence.
If you swap all instances Inversion for Preparation in the current
definition of RepetitionTimeInversion, and then say "For example, in an
MP2RAGE sequence this would be the time between successive inversion
pulses" I think it is clear.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/bids-standard/bep001/issues/4#issuecomment-440250096,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABD4zqyYWl9NE6kb4q0F8cYize8RTeIsks5uw-_zgaJpZM4WX6Zw
.

--
Gilles de Hollander
Postdoctoral Researcher
Department of Experimental and Applied Psychology, http://www.vupsy.nl/
https://webmail.login.vu.nl/OWA/redir.aspx?C=MeXKTnHrb7rqNpc5d9_z4HY5RYvDrp4dMHZjA7VgV9-renkmXN7UCA..&URL=http%3A%2F%2Fwww.vupsy.nl%2F
Faculty of Behavioural and Movement Sciences
Van der Boechorststraat 1, 1081 BT  Amsterdam | Room 1B75
website http://www.gillesdehollander.nl/ | github
https://github.com/Gilles86/


Android: Should reconnect to platform audioplayer on app resume.

On iOS, the audioplayer class can correctly reconnect to the platform audio player, but on Android, it does not.
To reproduce this issue:

Launch an app with an audioplayer instance and start the playback.
Use the "back" button to exit out of the app (background audio continues)
Start the app again.

RESULT: A second audioplayer instance will be created and a second audio stream will begin. The only way to stop the previous audio player is to us the Android System methods to kill or fully dismiss the app.

This is a problem I also noticed when you do a Full restart in debug mode. I tried for a bit to listen to this events on android, exploring a lot of documentation for the Registrar object, but could not find a proper method to hook. Do you have any idea on how to fix this? We'd be glad if you could help us with a PR. :)

I don't know Android development well enough to have a suggestion here. My workaround was to hook into the willPop method of the root navigator route and to stop playback whenever the app fully exits.

Hey guy I have a similar issue that when I press the home button to send app in background(without terminating it) or lock screen, audio stops. On unlocking screen or on bring it to the front makes it play again. Any suggestions?
Thanks in advance :)

Closing due inactivity


R70: Data Services: Monitoring: Monitoring services shall calculate the total number queries

Tools: MySQL

Apache Derby is the SQL tool used for now, as it doesn't require a server install, and we only need rudimentary, single-class, SQL functionality

Implemented and integrated in front end.

Completed.
SQA


Create Project Documentation

2 x Swagger (OB API, Internal RS API)
Project architecure overview docs
Postman collections
Testing doc
Operations Doc (monitoring, reporting etc)

Too vague to be useful now


Escape JS string backticks

Closes #82, depends #91 (to avoid conflicts).
Note that master now always uses backticks, so issues with escaping single quotes in the current release now manifest as issues with escaping backticks.
I think JSX doesn't need escaping because both {} and <> are fully used (or rejected for syntax violation during parsing) for ICU itself.

so issues with escaping single quotes in the current release now manifest as issues with escaping backticks.

Should we open another issue if we're closing https://github.com/unsplash/intlc/issues/82?


Improved  design system

Need to improve this component on \<Feedback-Form>\src\<feedback.design-system.mdx>
Improvements required :
Any reference image?
Any external reference?
You can checkout this one for better understanding.

@Rajdip019 Please assign me


[WIP] Fix maintable load failure in concurrent load and compaction sceneario

Why is this PR needed?
What changes were proposed in this PR?
Does this PR introduce any user interface change?

No
Yes. (please explain the change and update document)

Is any new testcase added?

No
Yes

Build Success with Spark 2.3.4, Please check CI http://<IP_ADDRESS>:12545/job/ApacheCarbonPRBuilder2.3/3320/

Build Success with Spark 2.4.5, Please check CI http://<IP_ADDRESS>:12545/job/ApacheCarbon_PR_Builder_2.4.5/1581/

combined in #3810


Using torch-dataframe for time-series

First of all I would like to congratulate you for this great project.
I would like to know if it is possible to use the torch-dataframe for time series study.
Something similar to xts in the R.
Example:
TimeSeries1:
| Date                             |       Values1      |
| 2016-12-27 21:00:00   |         10.00        |
| 2016-12-27 21:01:00   |         10.01        |
| 2016-12-27 21:02:00   |         10.02        |
| 2016-12-27 21:04:00   |         10.04        |
| 2016-12-27 21:07:00   |         10.07        |
TimeSeries2:
| Date                             |      Values2       |
| 2016-12-27 21:00:00   |         20.00        |
| 2016-12-27 21:01:00   |         20.01        |
| 2016-12-27 21:03:00   |         20.03        |
| 2016-12-27 21:05:00   |         20.05        |
| 2016-12-27 21:06:00   |         20.06        |
| 2016-12-27 21:07:00   |         20.07        |
Merge result of TimeSeries1 with TimeSeries2:
| Date                             |       Values1      |       Values2     |
| 2016-12-27 21:00:00   |         10.00        |        20.00        |
| 2016-12-27 21:01:00   |         10.01        |        20.01        |
| 2016-12-27 21:02:00   |         10.02        |          NA          |
| 2016-12-27 21:03:00   |            NA         |        20.03        |
| 2016-12-27 21:04:00   |         10.04        |           NA         |
| 2016-12-27 21:05:00   |            NA         |        20.05        |
| 2016-12-27 21:06:00   |            NA         |        20.06        |
| 2016-12-27 21:07:00   |         10.07        |        20.07        |
Applying na.locf to the merged TimeSeries
| Date                             |       Values1      |       Values2     |
| 2016-12-27 21:00:00   |         10.00        |        20.00        |
| 2016-12-27 21:01:00   |         10.01        |        20.01        |
| 2016-12-27 21:02:00   |         10.02        |        20.01        |
| 2016-12-27 21:03:00   |         10.02        |        20.03        |
| 2016-12-27 21:04:00   |         10.04        |        20.03        |
| 2016-12-27 21:05:00   |         10.04        |        20.05        |
| 2016-12-27 21:06:00   |         10.04        |        20.06        |
| 2016-12-27 21:07:00   |         10.07        |        20.07        |
Applying na.omit to the merged TimeSeries
| Date                             |       Values1      |       Values2     |
| 2016-12-27 21:00:00   |         10.00        |        20.00        |
| 2016-12-27 21:01:00   |         10.01        |        20.01        |
| 2016-12-27 21:07:00   |         10.07        |        20.07        |
Very Thanks
Danilo

This isn't available but should be easy to implement. All the positions of NA:s can be easily identified as they're stored as a tds.Hash with {id: true} structure, i.e. all you need to find is the id number and find the element immediately before and use that. Take a look at the Dataseries, you're welcome to add the functionality if you want to. Remember to write specs together with the functionality.

Good afternoon,
I also believe it will be easy to implement the locf function.
I will try.
But as I am still learning about the project, if possible, I would be grateful if you could show me how the two timeseries would merge together according to the index column (dateTime). Just a little example if its  possible.
Very thanks
Danilo

Great, start with writing a spec for the merge with the two dataframes and the desired outcome. I can then try to help you with the details of putting it together.

require 'Dataframe'
df1 = Dataframe()
date1 = {
"2016-12-27 21:00:00",
"2016-12-27 21:01:00",
"2016-12-27 21:02:00",
"2016-12-27 21:04:00",
"2016-12-27 21:07:00"
}
value1 = {
10.00,
10.01,
10.02,
10.04,
10.07
}
df1:load_table{data=Df_Dict{date=date1, priceA=value1}}
df2 = Dataframe()
date2 = {
"2016-12-27 21:00:00",
"2016-12-27 21:01:00",
"2016-12-27 21:03:00",
"2016-12-27 21:05:00",
"2016-12-27 21:06:00",
"2016-12-27 21:07:00"
}
value2 = {
20.00,
20.01,
20.03,
20.05,
20.06,
20.07
}
df2:load_table{data=Df_Dict{date=date2, priceB=value2}}

Ok, so you want to do a full join. There is one main problem and that is that no-matter how clever our implementation feels it will most likely be inefficient compared to other SQL-solutions that have been at it for years. My general design thought regarding torch-dataframe is to allow simple manipulations and some other stuff that's good to have for building and training models. Implementing hard-core joins has therefore not been something that I've aimed at. I personally prepare my datasets in R and then export them to CSV before importing to Torch. R has the dplyr-package that is excellent for all kinds of merges etc.
Anyway if you still want to embark on implementing the merge then:

Create a timestamp-type. Add to the Dataseries a to_timestamp as a string will be terribly inefficient to work with. I know the to_categorical function is rather slow and this will be even worse - consider doing this in C if you have large datasets. There is a SO post that may be helpful.
Create a sort for both tables using torch.sort's second return value that retrieves the indexes of the sort. Note that you will need to mask the missing data when sorting and appending the missing elements at the end.
The full_join function should

Sort both tables and find the number of mismatches
Create new Dataseries of correct type (or Dataframe using add_column) that can contain the merged dataset
Loop through both frames at once and fill in the values in the Dataseries previously created. There will be three indexes: index for first dataframe, index for second dataframe and index for the output series/frame
Create and return a new Dataframe with the Dataseries (you could possibly start with creating a new Dataframe of correct size and setting its elements)

That's it. A few hours of work though :-P


[LET-1442] Disable flight mode on map interactivity controls (panning & rotation)

This PR aims to disable map rotation and tilting by default.
Tracking
LET-1442

@SARodrigues Can you review the change I submitted (if you have access to a computer that do not use MacOS)?


avoid building latest image in relase branch

Signed-off-by: Zhiwei Yin<EMAIL_ADDRESS>

/assign @qiujian16

/approve
/lgtm


New segmentation model: GM and WM for exvivo DWI data (University of Queensland)

This PR integrates a new segmentation model to sct_deepseg.
This model has been trained on MRI exvivo DWI data provided by @ihattan2020 (University of Queensland - Center of Advanced Imaging). Model was trained with ivadomed, including the following features: 2D unet, multiclass (grey and white matters), softseg.
Fixes #3004.

Associated model repository: https://github.com/ivadomed/model_seg_mice_gm-wm_dwi_unet2d-multichannel-softseg

The description overflows in the other column. I changed the width to make the description fit here: 189a722

the recommended terminal width is 100, and currently this is even too small for deepseg description:

so instead of further extending columns (we should aim at the opposite), i would suggest to squeeze the description https://github.com/neuropoly/spinalcordtoolbox/pull/3224#discussion_r578705217

When trying the model I get this error:
Traceback (most recent call last):
  File "/home/charley/sct-dev/spinalcordtoolbox/scripts/sct_deepseg.py", line 245, in <module>
    main(sys.argv[1:])
  File "/home/charley/sct-dev/spinalcordtoolbox/scripts/sct_deepseg.py", line 207, in main
    nii_lst, target_lst = imed_inference.segment_volume(path_model, input_filenames, options=options)
  File "/home/charley/sct-dev/python/envs/venv_sct/lib/python3.6/site-packages/ivadomed/inference.py", line 255, in segment_volume
    preds = model(img) if fname_model.endswith('.pt') else onnx_inference(fname_model, img)
  File "/home/charley/sct-dev/python/envs/venv_sct/lib/python3.6/site-packages/ivadomed/inference.py", line 368, in onnx_inference
    ort_session = onnxruntime.InferenceSession(model_path)
  File "/home/charley/sct-dev/python/envs/venv_sct/lib/python3.6/site-packages/onnxruntime/capi/session.py", line 158, in __init__
    self._load_model(providers or [])
  File "/home/charley/sct-dev/python/envs/venv_sct/lib/python3.6/site-packages/onnxruntime/capi/session.py", line 177, in _load_model
    self._sess.load_model(providers)
onnxruntime.capi.onnxruntime_pybind11_state.InvalidGraph: [ONNXRuntimeError] : 10 : INVALID_GRAPH : This is an invalid model. Type Error: Type 'tensor(bool)' of input parameter (281) of operator (ScatterND) in node (ScatterND_160) is invalid.
Traceback (most recent call last):
  File "/home/charley/sct-dev/spinalcordtoolbox/scripts/sct_deepseg.py", line 245, in <module>
    main(sys.argv[1:])
  File "/home/charley/sct-dev/spinalcordtoolbox/scripts/sct_deepseg.py", line 207, in main
    nii_lst, target_lst = imed_inference.segment_volume(path_model, input_filenames, options=options)
  File "/home/charley/sct-dev/python/envs/venv_sct/lib/python3.6/site-packages/ivadomed/inference.py", line 255, in segment_volume
    preds = model(img) if fname_model.endswith('.pt') else onnx_inference(fname_model, img)
  File "/home/charley/sct-dev/python/envs/venv_sct/lib/python3.6/site-packages/ivadomed/inference.py", line 368, in onnx_inference
    ort_session = onnxruntime.InferenceSession(model_path)
  File "/home/charley/sct-dev/python/envs/venv_sct/lib/python3.6/site-packages/onnxruntime/capi/session.py", line 158, in __init__
    self._load_model(providers or [])
  File "/home/charley/sct-dev/python/envs/venv_sct/lib/python3.6/site-packages/onnxruntime/capi/session.py", line 177, in _load_model
    self._sess.load_model(providers)
onnxruntime.capi.onnxruntime_pybind11_state.InvalidGraph: [ONNXRuntimeError] : 10 : INVALID_GRAPH : This is an invalid model. Type Error: Type 'tensor(bool)' of input parameter (281) of operator (ScatterND) in node (ScatterND_160) is invalid.

Trying to replace the onnx model by its pt version.

Update: I am restarting the training. Sorry about that.

Update: I am restarting the training. Sorry about that.

AH! The problem was actually coming from a bug in ivadomed, see PR, which is, I think, fixed.
Note: we may want to do a new ivadomed release before the SCT one in order to incorporate these changes :-)

Please do not merge this PR yet: still working on the model (release 20210220) --> once properly tested and uploaded on its repo, it will be good to go! Keep posted soon

Release done!


cant builtin Ojbect through spritesheet.csv and  spritesheet.png

when running this code, it doesn't work
@builtin ||= Pixelart::Spritesheet.read(  image_path, meta_path, width:  24, height: 24 )

!!! ERROR - attribute name is not unique:
#<Pixelart::Metadata::Sprite:0x00000215e69efe78
@id=302,
@more_names=[],
@name="3D Glasses",
@type="Attribute">
duplicate:
#<Pixelart::Metadata::Sprite:0x00000215e69f4040
@id=301,
@more_names=[],
@name="3D Glasses",
duplicate:

Please, post to the public forum => https://old.reddit.com/r/DIYPunkArt


Catch2 store fails if estimated time exceeds 1m

Using the catch2 framework to benchmark some tests I encountered the following issue:
If your total estimated time for the tests exceeds the 60s, then the display time switches to minutes. If this happens, then the store action fails.
I have adjusted the catch2 example in https://github.com/phbasler/github-action-benchmark/tree/master/examples/catch2  ( added a 0.7s sleep before the fib(10) call so that the 100 executions exceed the 60s.
In the CI workflow the "Build and run benchmarks with Catch2" part is successful, but the "Store benchmark results" sections yields the following error:
`2022-04-05T07:01:00.8358638Z ##[group]Run benchmark-action/github-action-benchmark@v1
2022-04-05T07:01:00.8358959Z with:
2022-04-05T07:01:00.8359184Z   name: Catch2 Benchmark
2022-04-05T07:01:00.8359428Z   tool: catch2
2022-04-05T07:01:00.8359724Z   output-file-path: examples/catch2/benchmark_result.txt
2022-04-05T07:01:00.8360710Z   github-token: ***
2022-04-05T07:01:00.8360988Z   auto-push: true
2022-04-05T07:01:00.8361230Z   alert-threshold: 200%
2022-04-05T07:01:00.8361470Z   comment-on-alert: true
2022-04-05T07:01:00.8361716Z   fail-on-alert: true
2022-04-05T07:01:00.8401114Z   alert-comment-cc-users: @bernedom,@ktrz
2022-04-05T07:01:00.8401461Z   gh-pages-branch: gh-pages
2022-04-05T07:01:00.8401784Z   benchmark-data-dir-path: dev/bench
2022-04-05T07:01:00.8402088Z   skip-fetch-gh-pages: false
2022-04-05T07:01:00.8402344Z   comment-always: false
2022-04-05T07:01:00.8402594Z   save-data-file: true
2022-04-05T07:01:00.8402834Z ##[endgroup]
2022-04-05T07:01:01.1730005Z ##[error]No benchmark found for bench suite. Possibly mangled output from Catch2:
Catch2_bench is a Catch v2.11.0 host application.
Run with -? for options

-------------------------------------------------------------------------------
Fibonacci
-------------------------------------------------------------------------------
/home/runner/work/github-action-benchmark/github-action-benchmark/examples/catch2/catch2_bench.cpp:8
...............................................................................

benchmark name                                  samples       iterations    estimated
                                                mean          low mean      high mean
                                                std dev       low std dev   high std dev
-------------------------------------------------------------------------------
Fibonacci 10                                              100             1     1.16689 m
                                                   700.121 ms    700.118 ms    700.124 ms
                                                    16.061 us     14.141 us     18.543 us

Fibonacci 20                                              100             2      6.051 ms
                                                    30.137 us     29.714 us     32.152 us
                                                     4.019 us         80 ns      9.574 us

===============================================================================
test cases: 1 | 1 passed
assertions: - none -`

Thank you @phbasler for filing an issue. I'll try to fix it soon


Version 1.05+ stops working with Local?

I appears that 1.05+ stops working with MacOS Local here's the error:

When I roll back to 1.04 everything works fine without any changes to the environments.
This seems to be happening across the board within this type of environment.

can you please try latest version? 1.0.8

It doesn't work. No version between 1.0.5 and 1.0.8 work.
But 1.0.4 and below do work. All versions that don't work give the exact same error. The way the error shows:
Could not open input file: /Users/#####/Local

Makes me think that it's not escaping the spacing in the folders e.g. /users/#####/Local\ Sites/sitename/app/etc

It might have to do with the php-cs-fixer binary.
The different between 1.0.4 and 1.0.5 is that i have upgraded the php-cs-fixer binary from v3.28.0 to v3.59.3. Can you try to download v3.28.0 and pass it as toolPath to extension and see if it works for you?


Formatters: format signature incompatible with normalize signature

The format wait for an array (it's mandatory)
The normalize signature is more permissive, and hopefully, because there are instanceof tests in it
But format method calls normalize, so the $record is converted everytime to an array, and the instanceof tests aren't used anymore.
I have a case in Drupal8, with an Exception and its huge stacktrace. As the exception is convert to an array, there is an infinite loop or something like that because the normalize tests aren't used, only the if is_array($data). (By the way, it needs a protection system, as suggested by #924)
What could we do to change that ?
Many thanks !
Example on LineFormatter.php:
   /**
     * {@inheritdoc}
     */
    public function format(array $record)
    {
        $vars = parent::format($record);
        //...
    }

Mmmmh, seems to be linked to #528
In my case, Drupal generates a huge backtrace in an array (Drupal\Core\Utility::decodeException())
I tried to limit the backtrace array to 10 elements, and Monolog accept to display my message in a line, probably because the generated String isn't too big.
But if monolog will handle the $record type correctly, it could handle this kind of call, even if it's not its responsibility at first...

normalize is more permissive, because it is called recursively.
format is called with a Monolog record array, i.e. the basic structure used by Monolog internally (which would be represented as a struct in C, or as a shape in Hack)

it makes sense :) I understand what you mean.
I gonna fix the issue in Drupal.
Thanks @stof


Morthal Legate Tells Me I'm Trespassing

Ran into him in the street and he warned me I was trespassing. Not sure if this is something from this mod or not.

Should be fixed in https://github.com/schofida/CWORR/releases/tag/v1.0.11a


Styled scrollbar is affecting the accessibility of the website

Styled scrollbar is confusing and could affect the accessibility of the website. Maybe, we should revert it back to normal

Hey, I would love to work on this issue for hacktoberfest

Okay, I actually changed it because it looked bad on Electron. Feel free to update the scrollbar.
This issue is very simple to fix so I'll not be assigning it hacktoberfest tag.
Let me know if you are still willing to work on this.

Hey! I can do this. Should I change the style of the scrollbar to something similar but still a custom one to keep with the color schema and style of the website?

Yes, try to keep it so that it looks good in electron builds as well. Also please make sure to use CSS variables, all variables can be found inside src/assets/base.css. Feel free to be creative just make sure it doesn't stand out too much and fits the overall look of the website.
To test on electron do the following:

Run npm run build,
Run npm run electron:dev

@pratyushtiwary Trying to test my changes, but is there any way to test my changes live as opposed to running npm run build and npm run electron:dev everytime I want to see my style changes? It is getting quite tedious to build everytime I change colors or sizes

You can test it directly on your browser using npm run dev, if it looks fine there it'll look fine in electron as well.


Fix policy loop rate. Increase PD gains to reduce sim2real gap

Run the control loop every 0.01s (or whatever determined by action_repeat) via busy waiting.
Revert pd gains to kp=50, kd=5. Gives much more realistic sim2real than kp=16, kd=2.

In the future, the busy-waiting used to set the control loop rate should be replaced by asyncio timers or something else more efficient.

Merging without review so that Jaden can use it.


Version 2.0.4 incompatible with IntelliJ 2017.2.2

Hi,
I just updated IntelliJ IDEA to the last update 2017.2.2 and during restart it says

The following plugins are incompatible with the current IDE build: .ignore

Thank you

IntelliJ notified me just today about the update and I didn't notice thaere were altready an issue opened for this since 2 weeks.


Azure Storage connection string via env var

Description
I'm in the throws of creating a solution consisting of several microservices (all F#) running in docker (docker cloud) using your Type Provider, Akka.NET and Nats.io. All working as designed.  Messages flowing through wonderfully from Azure Storage Queue to consumers and worker nodes.  What I'm not sure on at this stage is my strategy on how to handle the connection string to Azure Storage with your TP.  I'm using connectionString permutation in app.config at the moment as seen here:
type FromConfig = AzureTypeProvider<connectionStringName = "myConnection", configFileName="sample.config">

and having to use a private repo in docker hub.  The pattern that I've seen most using with sensitive info is environment variables (supplied at container runtime).
Is the option of using env vars possible or is this on your road map?  Any advice welcome!
Warmest regards,
Garrard.

You have a few options. One is to manually override on every call, although that's probably undesirable. What you might want to look at (although this is somewhat hacky) is rewriting the connection string values right at the start of the application (before the TP initialises) from the environment. I've done something similar for hosting F# apps within the Azure App Service here.
To be honest I don't know if that's even possible, so if you can try it out and see if that's a viable solution - if not, we can look at adding environment variable support to the TP as well.

Thanks @isaacabraham, I will try this in a short while.
Warmest regards,
Garrard.

Hi @isaacabraham,
Thanks for giving me something to try.
I didn't get anywhere with it unfortunately. I'm now thinking of a hack. I'm going to pass an env var when I run the container in the normal way -e "foo=baa". The CMD ["",...] will invoke a script that will (a) insert env var value (connectionString) into app.config and (b) execute mono ./mymicroservice.exe.  In theory it should work.  I will just need to make sure that the queue name in dev/test/prod is named the same!
It's not ideal but it should work.
Warmest regards,
Garrard.

OK - perhaps environment variables is something that we should consider in future as well (though I'm not aware of any other TPs etc. supporting them).