Add CertificateRequest as a source

A common pattern to build a locally trusted CA with cert-manager is to create a self-signed issuer, use it to issue a CA certificate, then use that certificate with a ca issuer to issue leaf certificates. A common request with this configuration is to be able to supply a trust bundle to clients so they can trust the CA, and support rotation of that CA.
When a certificate is issued by cert-manager, it will create a CertificateRequest, whose status is populated with the public certificate that is issued. By default, this resource is not automatically garbage collected unless the Certificate object is deleted. The certificate requests are linked to the certificate via owner references.
By adding support for certificate requests as trust-manager sources, an end user could easily build a trust bundle for every certificate issued by the self-signed issuer, automatically extending it as the root certificate is rotated. If a specific issued root certificate is considered compromised, the user could delete the specific CertificateRequest and it would be removed from the trust bundle.
Suggested API:
spec:
  sources:
  - certificateRequests:
      certificate: "my-self-signed-ca"

The trust-manager would use owner references to identify the matching certificate requests (similar to how cert-manager does it in the issuing flow) , and use the certificate field from the status of the request to populate the trust bundle.
Alternatively, we could extend cert-manager to provide a CertificateRequest template option to be able to label requests, and select them that way.
We could also include the ability to select a specific CertificateRequest, but I'm not sure why we would do that.

Thanks for taking the time to raise an issue :grin:
I worry that CertificateRequests are significantly more disposable than you've listed here - many orgs choose to garbage collect then with some regularity.
Even if they weren't at risk of being garbage collected, I'm squeamish about relying on their information being correct. We know it's safe for a self signed cert, but I can't think of any other situation/issuer where it would be safe. That's because SelfSigned issuers are weird and probably shouldn't exist (there should instead be a selfSigned: true field on Certificate resources).
If the only reason to do this would be for SelfSigned issuers, I'm inlined to say that we're not solving the right problem by adding CertificateRequests as sources :thinking: What do you think?

many orgs choose to garbage collect then with some regularity.

I agree that orgs choose to garbage collect them, and there is an option to have cert-manager garbage collect older revisions too, but the key word here is choose. If an organisation decided to rely on these for their trust bundle, then they would choose to not garbage collect them, or to garbage collect differently (perhaps on expiry).

SelfSigned issuers are weird and probably shouldn't exist

I sort of agree here, but also understand that they exist because of the cert flow - by having them we keep the flow and the responsible controllers the same for all issuance, instead of having to treat them as a special case. Either way - this proposal doesn't actually involve the issuer at all, we're trusting revisions of a certificate.

If the only reason to do this would be for SelfSigned issuers, I'm inlined to say that we're not solving the right problem by adding CertificateRequests as sources thinking What do you think?

I agree that this solution is targeted at self signed issuers, with other issuers the root ca certs that need to be in a bundle can't be discovered in the same way.
I've seen numerous requests for a way to handle root rotation in these scenarios. Many solutions I've seen proposed end up being a little convoluted and only work in specific cases, I thought this suggestion might be a composable way satisfy the need using a fairly generic mechanism that already exists.
I can think of one scenario where restricting to the self-signed issuer is not necessarily the right thing to do. For example, lets say we have a root CA for an organisation, and we use that to sign intermediate CAs that are for specific teams/services. Its possible that an application would want to recognise certificates for a specific team/service but not for the organisation as a whole, so would want to use this on a CA issuer to build the trust bundle based on the intermediate not the root. I don't know of anyone doing this (I expect subject checking is more sensible).
Restricting to self-signed
If we want to restrict this to the self-signed issuer, we could potentially update the issuer instead with a field like PublicCertificateHistory: true so that all issued certificates are copied automatically into ConfigMap objects that can then be collected by trust-manager. That could help make using this pattern more explicit and stop users trying to do this with other issuers, but could confuse users as multiple Certificates using the same issuer would end up being in the same trust bundle automatically.
We could also recognise the use case as a more first class use case and create a new issuer SelfSignedCA that exhibits the above behaviour. That feels like a good way to make the user think about what they're doing, but feels more complicated than it needs to be.
The other way to restrict it to the self-signed issuer would be to check the issuer type when building the bundle, and refuse (or add a condition) when its not the self-signed issuer.
As an aside: The first iteration of this I thought of was to add a field to the Certificate resource called something like PublicCertificatePrefix, and on issuing a certificate, to also create a ConfigMap named <prefix>-<revision> containing the public cert, user specified labels, and no owner references. Then we could add label selectors to trust-manager to collect those into a bundle based on those labels. It was while I was investigating how that would be implemented that I found that CertificateRequest resources already follow the same pattern.

On the Proposal
This is the only bit that really matters, but I added some more thoughts below!

If we want to restrict this to the self-signed issuer, we could potentially update the issuer instead with a field like PublicCertificateHistory: true...

That actually seems like a really attractive option. It applies nicely to all certificates, and it seems like something people would want for different certs too. It helps with this kind of trust-manager problem, but also helps for auditing purposes (e.g. "I've had 30 different Let's Encrypt certs on this cluster, here they all are").
I'd actually add it to the Certificate resource rather than an issuer, I think.
Unfortunately, I'm not sure if any of the maintainers would have time to implement (and it's possible that there might be blockers to implementing it which I haven't thought of!).
On Trusting Intermediates

I can think of one scenario where restricting to the self-signed issuer is not necessarily the right thing to do. For example, lets say we have a root CA for an organisation, and we use that to sign intermediate CAs that are for specific teams/services.

I tend to think that this is an antipattern and I'd advise against it if I saw it, because this turns the structure of an org's PKI into an implicit auth decision.
It's so easy for a team to trust the root certificate and have everything work, only to discover later that trusting the root means trusting everything. It also makes rotation much more difficult (because every intermediate has to be treated carefully).
This little bit isn't really relevant to the problem we're discussing, but I find that I weirdly care about PKI and I like talking about it :joy:
On SelfSigned

I sort of agree here, but also understand that they exist because of the cert flow - by having them we keep the flow and the responsible controllers the same for all issuance, instead of having to treat them as a special case. Either way - this proposal doesn't actually involve the issuer at all, we're trusting revisions of a certificate.

My thing is that SelfSigned is already a special case which has to be treated differently, because it's the only issuer where the issuer needs to know the private key of the cert to be signed. All other issuers use CSRs. SelfSigned is already implemented in a weird way where the private key has to be shared with the issuer!
You're totally correct that the proposal doesn't involve the issuer, of course! My reason for bringing up the weirdness of SelfSigned is that I tend to be wary of anything which treats it as more of a special case. Because it's weird, if I see a proposal which seems to apply mostly to SelfSigned I'm extra careful!

That actually seems like a really attractive option. It applies nicely to all certificates, and it seems like something people would want for different certs too. It helps with this kind of trust-manager problem, but also helps for auditing purposes (e.g. "I've had 30 different Let's Encrypt certs on this cluster, here they all are").
I'd actually add it to the Certificate resource rather than an issuer, I think.

That was my first thought, to add that to the Certificate. It was as I was looking at the code to see how complicated it would be that I realised that CSRs already had the required data in them. Those CSRs could also already be used for audit purposes if they aren't cleaned up, and cleaning them up is an active decision that gets made by the user, so if they want to use them for audit, they just don't clean them up.
Why do you think it would be better to use a PublicCertificateHistory field that duplicates each public cert to a configmap, instead of using the existing CertificateSigningRequest that already follows that pattern? (I don't mind contributing it, I would probably open an issue on cert-manager first to get more opinions before writing code though, and that's a question I expect would come up!)
Also, I'm not expecting anyone to code it for me, I'm happy to contribute, but wanted to get some feedback before doing a lot of work. I have an untested changeset for trust-manager to pick up data from the CSR already, and have put some thought into how PublicCertificateHistory would need to change the Certificate issuing flow in cert-manager, but with no code behind it yet.

I tend to think that this is an antipattern and I'd advise against it if I saw it, because this turns the structure of an org's PKI into an implicit auth decision.

Yeah, you're right and my example was stretching. Your example (audit trail) is more appropriate.

Before I start a discussion over in the cert-manager repo, one question that might help me write that request:
Why do you think it would be better to use a PublicCertificateHistory field that duplicates each public cert to a configmap, instead of using the existing CertificateSigningRequest that already follows that pattern?

Hey! Sorry, been a crazy couple of weeks and I've been meaning to get back to this.
I guess the main reason I'd suggest a CertificateHistory field would be API design. CertificateHistory is incredibly specific, and it's not overloaded in any way. It would be clear to me that I could use that history ConfigMap to trust every iteration of a cert in trust-manager, for example.
CertificateRequest objects are used for requesting certs, and so using them for trust purposes would be overloading their purpose. They're also liable to being cleaned up / garbage collected - it's possible to imagine a workaround to prevent them being garbage collected, but equally it's possible to imagine a well-meaning engineer cleaning them up manually, not realising how they're being used.

Raised a suggestion in cert-manager here: https://github.com/cert-manager/cert-manager/issues/6224

I think that linking cert-manager with trust-manager in general is going to be a very useful thing. However I think there's another issue that is important and I don't think it's been touched on here yet.
There is a differentiation between which CA cert is being used to sign, and which CA cert(s) should be trusted. This is obvious during a rotation. You will need to start trusting the new CA before it is being used to sign, and stop trusting the old one after there are no more certs signed with it in use.
The CertificateHistory idea is starting to touch on this, but I think the solution is more complex than that allows.
Having automatic connection between cert-manager and trust-manager would be really useful, but I wonder if the use cases would be limited without something quite a bit more complex.

Having automatic connection between cert-manager and trust-manager would be really useful, but I wonder if the use cases would be limited without something quite a bit more complex.

Can you describe how you think the use cases would be limited?

Can you describe how you think the use cases would be limited?

If you just trust the current signing cert then you will have issues at rotation time. If you trust all historical certs then you are trusting more than you want to ideally. These two extremes may work for some use cases, but anything more nuanced needs a more complex approach IMO.

Right, so you think there needs to be a way to determine when all the certs issued by a CA have expired? That's a difficult question to answer :)
Trusting an expired CA isn't ideal, but shouldn't typically be unsafe.

Trusting an expired CA isn't ideal, but shouldn't typically be unsafe.

That could actually be a useful standalone trust-manager improvement. There's no reason to include expired CAs in our output I can think of (unless people are running massively different clocks on different machines, but I don't think that's worth us supporting).
I think we could filter anything expired out of the ConfigMap we generate.
In any case,  I think you're correct that it's safe. If an attacker had the ability to modify the time on a box, they could maybe make use of an expired cert - but I think it's reasonable to assume that attackers cannot modify time in our threat model (AFAIK changing time requires privileged access in most cases, so it's already game over if they can do that)
The issue really is distrusting a still-valid cert that's been rotated. James makes a good point that PublicCertificateHistory makes it very easy to still trust a cert that's been rotated.

Right, so you think there needs to be a way to determine when all the certs issued by a CA have expired? That's a difficult question to answer :)

I think more generally, it would be required to find out when all certs issued by a CA were no longer used, with it being a safe assumption that an expired cert is no longer being used (and that no cert issued by an expired CA is being used).

That could actually be a useful standalone trust-manager improvement. There's no reason to include expired CAs in our output I can think of...

I believe a cert is valid as long as it was signed by the CA while it was valid, and that an expired CA can indeed be used as part of a valid trust chain.
I don't think we can just not include any expired certs :(

I believe a cert is valid as long as it was signed by the CA while it was valid, and that an expired CA can indeed be used as part of a valid trust chain.

The first part of this surprised me! I definitely agree that an expired CA can be used in a valid chain, but I think it would have no effect. I tested this by writing a program which generates a root and leaf certificate.
First I generated a root which hadn't expired to test that the chain was valid. This test was done before 2023-07-31T10:00:00Z.
# Root cert:
# Validity
#   Not Before: Jul 31 09:37:36 2022 GMT
#   Not After : Aug 10 09:37:36 2023 GMT
-----BEGIN CERTIFICATE-----
MIIBBjCBuaADAgECAgIFOTAFBgMrZXAwETEPMA0GA1UEChMGdGVzdE9VMB4XDTIy
MDczMTA5MzczNloXDTIzMDgxMDA5MzczNlowETEPMA0GA1UEChMGdGVzdE9VMCow
BQYDK2VwAyEA9i8wVyrym754KyvVasojACJNBYELhbBaA54jxi1yJp2jNTAzMBIG
A1UdEwEB/wQIMAYBAf8CAQIwHQYDVR0OBBYEFHb9oGnwr7cm0df4WBuLfTD8H7qi
MAUGAytlcANBAIZHRZDfzCLmU5vnwr3Q+vMoq6al6E//G/EQ3WfgxZKs1lndiCv5
Y03GwGoKVfvM7EZxVlaF3GXWqLBR/AuOUAs=
-----END CERTIFICATE-----

# Leaf cert
# Validity
#   Not Before: Jul 26 09:37:36 2023 GMT
#   Not After : Aug  5 09:37:36 2023 GMT
-----BEGIN CERTIFICATE-----
MIHrMIGeoAMCAQICAhyjMAUGAytlcDARMQ8wDQYDVQQKEwZ0ZXN0T1UwHhcNMjMw
NzI2MDkzNzM2WhcNMjMwODA1MDkzNzM2WjARMQ8wDQYDVQQKEwZ0ZXN0T1UwKjAF
BgMrZXADIQCClq2g4Io7jhtThwL4Lp/rhkWmP7J6CslB34Rm0teMUqMaMBgwFgYD
VR0RBA8wDYILZXhhbXBsZS5jb20wBQYDK2VwA0EAeslRHiTu/W853rWQUdU04Sxd
GdFtvHZNVLrBnE0IA1UwCldCuIRuuhApXBFrVg/Jcnu0vmQCWdmJM43T0XuSAQ==
-----END CERTIFICATE-----

I ran openssl verify to check this was valid, and it was:
$ openssl verify -CAfile root.pem chain.pem
chain.pem: OK

Then I regenerated the chain with a root cert which had expired but with a leaf cert which was still unexpired:
# Root cert
# Validity
#   Not Before: Jul 31 09:41:14 2022 GMT
#   Not After : Jul 21 09:41:14 2023 GMT
-----BEGIN CERTIFICATE-----
MIIBBjCBuaADAgECAgIFOTAFBgMrZXAwETEPMA0GA1UEChMGdGVzdE9VMB4XDTIy
MDczMTA5NDExNFoXDTIzMDcyMTA5NDExNFowETEPMA0GA1UEChMGdGVzdE9VMCow
BQYDK2VwAyEAJ7fX2joHtI/uV/eVFGArjjNZcSCV7GSVYNUdZ8MSQEijNTAzMBIG
A1UdEwEB/wQIMAYBAf8CAQIwHQYDVR0OBBYEFHVc2w1ggTX4qWr3x23xobwQYzk8
MAUGAytlcANBALoMDsLTbusM6qZm8Op+i6Wv+972pYf3S6ZUexv6FUPAbG9/K/6j
pIWec/Ll4IpIE7kfsUNhBvZDPR+0JFEnDAg=
-----END CERTIFICATE-----

# Leaf cert
# Validity
#   Not Before: Jul 26 09:41:14 2023 GMT
#   Not After : Aug  5 09:41:14 2023 GMT
-----BEGIN CERTIFICATE-----
MIHrMIGeoAMCAQICAhyjMAUGAytlcDARMQ8wDQYDVQQKEwZ0ZXN0T1UwHhcNMjMw
NzI2MDk0MTE0WhcNMjMwODA1MDk0MTE0WjARMQ8wDQYDVQQKEwZ0ZXN0T1UwKjAF
BgMrZXADIQAx6lQrUzwFtoIJIAqX5OuPCbGmWXNMGU1zAtgPuJFAkqMaMBgwFgYD
VR0RBA8wDYILZXhhbXBsZS5jb20wBQYDK2VwA0EAaY49vM7wGBOgu7ubD13yHsPz
rdKiG/aN6ywSgw5QThIxprSomK28mPaaO/wILWgbzVE4+0WNJB4DrpbNwfSmCw==
-----END CERTIFICATE-----

Then I tried to verify again:
$ openssl verify -CAfile root.pem chain.pem
O = testOU
error 10 at 0 depth lookup: certificate has expired
error chain.pem: verification failed

I'm now convinced that if a CA in the chain has expired, the leaf will not verify!

In openssl :)
It's not in the spec, and the signature is based on the CA key. There is nothing to stop a new ca cert being made with the new key, afaik.
Good post on the subject here: https://security.stackexchange.com/questions/120469/can-a-ssl-certificate-have-longer-validity-period-than-its-parent-in-x-509-chain
I think it would make a good addition to the spec.

Have improved my understanding and this is indeed wrong:

I believe a cert is valid as long as it was signed by the CA while it was valid, and that an expired CA can indeed be used as part of a valid trust chain.

The spec just doesn't say the validity period needs to nest. It does say all the certs need to be valid at the time of validation.
The leaf could still be valid, but would require a new ca cert with the same key to validate a whole chain.
So yes, I think we could not include expired CA certs in a trust bundle.


Experiment: try moving cmd to submodule

As part of the package renames, move packages or groups of packages to submodules to decrease the transitive the dependencies of the main module. This can be done for packages which will not need to be dependencies of the main module.

The fuzz issue has been resolved, marking this as ready for review

I'm going to close this one for now as it is easier to do the main module migration without submodule.


drivers: modem: update altair modem

Summary
update Altair modem with the ALT1250 implementation
Impact
LTE driver
Testing
Spresense

@pkarashchenko thanks for reviewing the code

@xiaoxiang781216 @pkarashchenko anything else?

@xiaoxiang781216 @pkarashchenko anything else?

If you can address my comments for double include guard in header files it would be great

@xiaoxiang781216 @pkarashchenko anything else?

@xiaoxiang781216 @pkarashchenko anything else?

If you can address my comments for double include guard in header files it would be great

I am fairly sure that I removed those but I do not have a resolve button for them. please check the code

Please fix
modem/alt1250/alt1250.c: In function 'alt1250_open':
Error: modem/alt1250/alt1250.c:1010:7: error: implicit declaration of function 'nxmutex' [-Werror=implicit-function-declaration]
 1010 |       nxmutex(&dev->waitlist.lock, 0, 1);

@xiaoxiang781216 @acassis would you like to take a look?

LGTM.


Why can't I get a value using sonic.LangEng in search.Query

As the title says
https://github.com/expectedsh/go-sonic/blob/master/cmd/example/main.go#L52
Why can't I get the value?
Is it due to sonic bug?
please let me know if anyone knows, thanks.

yea, I am also getting an empty result... but if I searched using the keyword "Batman" it returned a result of "id:1m2n3b4vf6"...
Not sure what's really happening behind the search...
tried as well through the Sonic terminal using telnet and here are the results that I got

START search SecretPassword
STARTED search protocol(1) buffer(20000)
QUERY movies general "man" LIMIT(10)
PENDING SPUHA0zl
EVENT QUERY SPUHA0zl
QUEY movies general "batman" LIMIT(10)
ERR unknown_command
QUERY movies general "batman" LIMIT(10)
PENDING 30oFMLVW
EVENT QUERY 30oFMLVW id:1m2n3b4vf6
QUERY movies general "is another" LIMIT(10)
PENDING 16601Qzh
EVENT QUERY 16601Qzh
QUERY movies general "this is another" LIMIT(1)
PENDING 2tpSAvAW
EVENT QUERY 2tpSAvAW
QUERY movies general "this is another movie" LIMIT(1)
PENDING W2plriPD
EVENT QUERY W2plriPD id:68d96h5h9d0
QUERY movies general "this is another" LIMIT(10)
PENDING T3CZzqV0
EVENT QUERY T3CZzqV0
QUERY movies general "another" LIMIT(10)
PENDING 3i01gyRk
EVENT QUERY 3i01gyRk
QUIT
ENDED quit
Connection closed by foreign host.

Hello ! Sorry but the repository is not well maintained due to time, but @remicaumette and me are going to look at it soon.
Thanks for your patience.


Sporadic crash in destroyWindow() when using Vulkan

I mentioned this problem on Twitter (https://twitter.com/postgoodism/status/708466175890432001) and you volunteered to take a look at the call stack. I think it's time to take you up on that offer.
I've attached as minimal a repro case I can manage; it creates a vulkan instance, device, swapchain, initializes GLFW, and immediately shuts it all down without rendering anything. Every 3-4 runs, I get a crash in GLFW's destroyWindow() function inside my call to glfwTerminate(). I'm building with MSVC 2013 (project include), and linking against GLFW commit id 793eef1d0a7. OS is Windows 7 x64, GPU is NVIDIA GTX 860M (drivers 364.47).
vulkswagen.zip
One potential clue: my debug report callback reports this error when I create the swapchain:

ERROR: [Swapchain] Code 4 : vkCreateSwapchainKHR() called with pCreateInfo->surface that was not returned by vkGetPhysicalDeviceSurfaceSupportKHR() for the device.

When I skip swapchain creation entirely, I can't seem to reproduce the crash.

I can reproduce the error message but not the crash. Windows 7 64-bit, GTX 660M with 364.47, <IP_ADDRESS> SDK and runtime.

Also tested with the <IP_ADDRESS> SDK and runtime with the same result.

I managed to reproduce the swapchain error but i couldn't recreate the crash.
OS: Windows 10
GPU(s): AMD r9 270x and AMD r9 270 in crossfire
Driver: AMD Crimson 16.3 (beta)
Vulkan SDK: <IP_ADDRESS>

Nuts. Yeah, I can't reproduce it on my desktop PC either (same environment but with a GTX 680); just my laptop. Thanks for checking; I'll keep poking around.

Did you ever find the cause of this?

From my experience with Vulkan, the error is probably due to an issue with memory allocation. The error that Vulkan gives should not be the cause of the crash. Make sure that you don't under allocate memory anywhere in your code.

Nope, no sign of it in close to a year. I'd be fine with closing the issue.


Fix Filters.regex Javadoc

https://spruce.mongodb.com/version/5f4e52d3d6d80a0e00d2fa56/tasks

Merged on command line.


Styling not working in Firefox for polymer elements that extend elements

Originally opened as dart-lang/sdk#19841
This issue was originally filed by<EMAIL_ADDRESS>
When you create the default polymer sample web application in the Dart Editor, change the clickcounter element to extend the DivElement (in HTML and dart) and change the HTML where it is instantiated to this:

all the styling is lost in Firefox (30.0).
It works in Chrome.
Using polymer 0.11.0+5

 Comment by zoechi

I tried it. The issue is caused by Polymer not creating a style tag for a custom element that extends div.
I added this style tag to the <head> element manually and the styling worked again for the <div is='click-counter'> element.

This is the style tag as it was created for the unmodified <click-counter> tag when opened in Firefox (I just replaced click-counter with div[is='click-counter']

 Comment by dgrove

Added Pkg-Polymer, Triaged labels.

 Comment by zoechi

see also http://stackoverflow.com/questions/24580897

 Comment by zoechi

seems
to be this issue https://github.com/Polymer/polymer/issues/266
and a dupe of https://code.google.com/p/dart/issues/detail?id=18171

 Comment by sigmundch

Removed Priority-Unassigned label.
Added Priority-Medium, PolymerMilestone-Next labels.


VkFFT 1.3

Dear @vincefn,
I have made some substantial changes to VkFFT in version 1.3 (https://github.com/DTolm/VkFFT/tree/develop), so there will be a two-month period before it is merged in the main branch for people to adjust the dependent projects. Namely, VkFFT is no longer a header-only file, but rather a collection of headers. This should not increase the complexity of usage - you still link to vkFFT.h file and it includes the other files. The main advantage is that the code base now is more structured and way easier to understand by other developers.
I have tested the code on some systems with the implemented benchmark scripts - RTX2080 (Vulkan, CUDA, OpenCL), MI250 (HIP), A100 (CUDA), UHD610 (Level Zero) and M1 Pro (Metal), however, your suite is more thorough in this regard. Also, you might be interested in exploring the new design.
I suppose keeping an issue for this period can be helpful for discussion.
Best regards,
Dmitrii

Hi @DTolm , thanks for the heads-up, I'll try to adapt pyvkfft in a few days.
(PS: I was actually going to mail you because I found a performance regression on V100 (but not only) for some sizes, e.g. c2c 512**3 3D transform - almost 2x slower than in version 1.2.21)

Dear @DTolm (Cc @picca)
Maybe one quick comment regarding the organisation of the headers: splitting the different parts is a good thing, itwill be more usable that way.
However I do not know how you intend to have the headers distributed and used. Previoulsy, this just required adding one path and using #include "vkFFT.h". I see that for Debian the approach is similar given the files distributed (https://packages.debian.org/sid/all/libvkfft-dev/filelist).
Now you are using #include "vkFFT_Structs/vkFFT_Structs.h", which means that you expect a directory organization like:
INCLUDE_DIR/vkFFT.h
INCLUDE_DIR/vkFFT_AppManagement/vkFFTDeleteApp.h
INCLUDE_DIR/vkFFT_AppManagement/...
INCLUDE_DIR/vkFFT_CodeGen/Kernelsvel0/vkFFT_KernUtils.h
INCLUDE_DIR/vkFFT_CodeGen/...
INCLUDE_DIR/vkFFT_PlanManagement/...
INCLUDE_DIR/vkFFT_Structs/...

where INCLUDE_DIR would be e.g. /usr/include under linux.
This is a little unusual as it requires copying not one directory (vkFFT) with all the headers, but instead copying both vkFFT.h and the subdirectories vkFFT_AppManagement, vkFFT_CodeGen, vkFFT_PlanManagement and vkFFT_Structs into the distribution's include directory..
This is just a suggestion, but there would be two simpler ways of doing this:

do not change your directory structure, but include from one level up, i.e. use:

#include vkFFT/vkFFT.h
#include vkFFT/vkFFT_AppManagement/vkFFTDeleteApp.h

This has the advantage of only requiring a single copy or link of the vkFFT include folder.

simplify a bit more and use

INCLUDE_DIR/vkFFT.h
INCLUDE_DIR/vkFFT/AppManagement/DeleteApp.h
INCLUDE_DIR/vkFFT/AppManagement/...
INCLUDE_DIR/vkFFT/CodeGen/Kernelsvel0/KernUtils.h
INCLUDE_DIR/vkFFT/CodeGen/...
INCLUDE_DIR/vkFFT/PlanManagement/...
INCLUDE_DIR/vkFFT/Structs/...

I think the latter is a much more standard way of organising the include directories - and more convenient, with one top include header and one directory.
I've also put @picca in Cc who maintains the VkFFT Debian package for further advice.

I am happy to change the codebase directory layout to be more in line with other header-only libraries - so probably the second approach is the easiest?

I think this would be best. But please get another opinion before committing - I don't know if @picca has an opinion on this.

As for the performance regressions - it is likely related to experiments with the dispatch threadblock size. They can improve some systems while making other systems worse and it is hard to decide the best size automatically for all systems. Currently, this decision-maker is located on lines 447-773 in vkFFT_Plan_FFT file and should probably be moved to a separate file as well. I will check what happens with 512^3 system and try to make some improvements.

Could it make sense to make this tunable - changing the threadblock size using an optional parameter ? You could imagine using this like in FFTW with their FFTW_ESTIMATE and FFTW_MEASURE - it would be the task of the calling library (e.g. pyvkfft) to test the performance of different sizes. I don't know how deep this is in the code and so if that would be possible.
Maybe it's already tunable through e.g. maxThreadNum - let me know.

Regarding the speed issue, here are some comparison benchmarks:
The A2000 has no issues in 2D (at least up to 1024) but a decrease in 3D:
(below the graphs up to 1024 are 2D, up to 660 this is 3D transforms - all batched so the actual array transformed are about 1GB)

For Titan V and V100 the decrease is clearer:

That's unexpected, I will investigate the reason today and add the update to the 1.3.0 branch. Thanks for finding it out!

Hello, just about the include files. I prefer the 2nd solution.
you have to decide for your user what is the prefered include directive
#include <vkFFT.h>

Then you decide where the includes files are installed by default. (for example with the autotools, it  is under the includedir path  which is by default $DESTDIR/usr/include/, but you can also decide to intall under a versioned version of your library
something like
/usr/include/vkFFT-'X'/
/usr/include/vkFFT-'X'/vkFFt.h
/usr/include/vkFFT-'X'/vkFFT/...

This is the way gtk libraries are installed. Usually a library comes with a pkg-config files which allows to find the -I{dir} where the includes files are expected. This is particularly important if you select the versionned directory structure, because this directory is not part of the default include search paths.
then using pkg-config we have
pkg-config --cflags vkFFT
-I/usr/include/vkFFT-X/

pkg-config --libs vkFFT
<nothing> this is a pure header library

The install script should be compatible with this pkg-config files generated during the build.
I am not a specialist of cmake, but I think that this is identical.
for now I would keep the 2nd solution proposed by Vincent, since using two version of the library at the same time is not  something expected (I think)
do not hesitate to ask further question if I was not clear enough.
Frederic

Hi @picca - thanks for the feedback. I don't think versioning would be needed for VkFFT, so I guess solution 2) above would be better - and it does not change anything to the end user, just using #include <vkFFT.h> would still work.

Hi @DTolm - here are longer 2D benchmarks. The decrease in performance is largely localised in the length<1024 region:
Titan V:

For the V100 - I'm suprised at the spread compared to the Titan V:

The A2000 throughput is remarkably stable over a wide range:

Hi @DTolm, I have begun playing around to advanced VkFFT parameters to see if this could easily be used to bake more optimal plans.
There is now a pyvkfft-benchmark script which can be used to test those relatively easily.
Some preliminary interesting results (only using radix 2 & 3):
On my macbook's M1, lowering aimthreads to 32 (maybe 64 would be enough, 16 gives similar results) gives improvements in 2D and 3D:

On a Titan V, increasing coalescedMemory to 128 or 256 (for 3D) gives very nice improvements:

And finally for a V100 also tweaking coalescedMemory:

I like this very much - as you said finding optimal parameters can be very tricky given the diversity of GPU configurations, so searching for the best options could easily be done by a higher-level library like pyvkfft, if the user chooses to do so (like the FFTW_MEASURE approach). In my case where I use iterative algorithms this can make a lot of difference.

@vincefn This study is also very interesting from an understanding of the GPU perspective. I have added access to batching parameter - groupedBatch. Batching here means how many FFTs are done per single kernel by a dedicated threadblock. It will try to force the user value for batching for each of the three axes. It has an effect on how many threads are allocated, read/write pattern, coalescing, etc. I also attach a simple search routine that looks for an optimal parameter on a discrete grid of batches - it is slow and unoptimized, but still improves the results.
Below are the results for Nvidia A100:

In the text file, you can see the batching parameters that the routine has found.
batching_test_A100.txt
batching_test_A100.txt
sample_1003_benchmark_VkFFT_single_3d_2_512.zip

As for the increase of coalesced memory to 256b - it solves the issues of big systems (as they benefit from more batching), but will be detrimental to small systems (as some compute units will have no work). Also, it shortens the single upload size and as VkFFT which can also increase the memory transfers. So there has to be a logical explanation for when to batch more.

Thanks, I have added the groupedBatch parameter to the ones which can be tweaked. It will be more complicated to tune as the range of values is large (compared to simpler parameters like aimThreads and coalescedMemory).
Just to understand - this affects the batched FFTs, or the number of parallel 1D FFTs performed per block ? I mean, if the array is 50x512x512 and the transform is 2D, does this affect how the 512 1D transforms are distributed, or the 50 (batch dimension) ones ?
Here are the benchmark results tweaking just the coalescedMemory parameter:

In 50x512x512 case for 1st axis 512 length fft, it will determine how many of the 50x512 total ffts are executed per one threadblock (usually 1-16). It is also indirectly affected by coalescedMemory and aimThreads (but forcing groupedBatch bypasses that).

It will be relevant and by increasing the groupedBatch parameter for y and z axis you will solve the speed issue.

OK, it is slow but it works - at least using the cuda backend: with OpenCL (on an nvidia card), I easily end up with a CUDA_ERROR_INVALID_VALUE and program abortion, so I can't really test different configuration.
Results do not seem to be very sensitive to the X-axis batch parameter (I guess it makes sense when not strided).

I have not tested other backends with groupedBatch option, only CUDA one, will check what is wrong with it there. For X-axis it is indeed less noticeable (only for small systems - cubes up to 100) as bigger systems have good coalescing and thread assignment already.

Dear @DTolm - regarding the CUDA_ERROR_INVALID_VALUE  error with OpenCL, you can probably ignore that. I thinks this only happened with PoCL, which I normally don't test as it has problems handling all VkFFT kernels.
However I've begun the systematic tests on the current VkFFT develop branch (including https://github.com/DTolm/VkFFT/issues/112) and I have a few calculation errors (accuracy failures in my tests) with non-radix transforms, see for example on an A40 using cuda or OpenCL:
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-05-19-a40cl/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-05-19-a40cu/pyvkfft-test.html
There are also a number of VKFFT_ERROR_FAILED_TO_COMPILE_PROGRAM  for R2C transforms.
Otherwise I've introduced an auto-tuning option in pyvkfft to optimise parameters like aimThreads, coalescedMemory or warpSize and it seems to work well when preparting the VkFFTApp.
Here's an example in 2D on a V100:

or on a A100 in 3D:

In those cases I just test coalescedMemory at 32, 64 or 128, so it's cheap but effective :-)

Dear @vincefn
I think I have fixed the C2C/R2C issue by changing how thread indexing works in one of the edge cases.
As for the coalescedMemory tuning - can you share the results as a text file so I can try and generalize the findings? It is good that the code can be made faster by tweaking one number, but the runtime compilation is still an issue in some cases, so I am still not sure if an autotuner should be the default behavior.
Best regards,
Dmitrii

Hmm. actually the timeout is after 120s, not 10. So it's not a question of compilation time.
If I try without parallel processing (in this cas on a A2000) with --serial, i.e.:
pyvkfft-test --systematic --backend pycuda --gpu a2000 --max-nb-tests 0 --ndim 1 --range 4198 4400 --r2c --bluestein --norm 1 --serial
Then I get a segmentation fault when it gets to the R2C transform with size 4202. That's with a cuda driver 530.30.02 (cuda 12.1) and cuda toolkit 11.7.0 (but the error also appears with the A40 and cuda driver 11.7, same toolkit).

I see, I checked these sizes on rtx 2080 and you run on a 40 which is Ampere and has more shared memory. These sizes are within single upload limit on it and behave differently compared to Turing, I will fix them later today.
As for the tuning results, it would be best to have all results to compare the relative gains, thank you!
As for Rader's Transform, the formula is - if the sequence is decomposable as multiplication of primes with each prime satisfying that P-1 is decomposable as radix 2-13 multiplication or P being 47, 59 or 83 then it is Rader's algorithm (these are default values and can be tuned). And each prime must be less than a single upload size. Otherwise it is Bluestein's algorithm.

I have identified the incorrect register calculation issue in C2R and fixed it, but I have done it only with emulating Ampere architecture, so I only checked that the code compiles.

Looks good so far:
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-05-25-a40cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-05-25-a40cl/pyvkfft-test.html
..just 1 DCT2 failure of size 4 in OpenCL - but it may be a fluke, I'll retry it separately

OK - the cuda tests all passed on the A40, but there are some failures using OpenCL (also A40) with VKFFT_ERROR_FAILED_TO_COMPILE_PROGRAM.
I re-tested all failures with less parallel process separately, and you can see the log:
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-05-25-a40cl/log-6821099.txt
There are specific failures with messages: ptxas error   : Entry function 'VkFFT_main' uses too much shared data

The R2C issue has been fixed.

Thanks @DTolm . The OpenCL test do look OK (still ongoing):
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-06-01-a40cl/pyvkfft-test.html
Same for cuda:
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-06-01-a40cu/pyvkfft-test.html

Hi @DTolm - all the previous tests on nvidia cards (A40, V100, GTX 1080) have passed (some occasional memory errors but once re-done with lower parallelism all passed).
However I can now test an AMD Vega56 (gfx900) card, and there are some isolated errors with 1D transforms of size 7700
(C2C float32) and 7700 or 15400 (R2C float32).
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-06-26-vega56cl/pyvkfft-test.html
The test is still ongoing but I think that 7700 error is significant and not a fluke.

Besides the errors with sizes 7700/15400, there are errors with non-radix transforms (Rader I guess) for sizes like 3074, 3076, 3078,..
The gfx900 card actually is a Radeon Pro WX 9100

Hello,
I couldn't reproduce any of the mentioned errors with Radeon Pro VII on Windows, so it will really help if you can send the generated kernels code (with keepShaderCode = 1) for systems 7700 and 3074 in C2C; 7700 and 15400 in R2C? Then we can understand with certainty if it is a driver issue or a code issue.
Best regards,
Dmitrii

Thanks for looking into this !
I have attached the codes for the different transforms.
code.zip

@vincefn The issue is now fixed, it was related to AMD GPUs apparently allowing only 256 threads per kernel on Linux OpenCL which was scaled incorrectly in some cases before. It was all combined with the fact that Windows and Linux AMD OpenCL drivers also report different max shared memory values (32KB and 64KB respectively). Vulkan can use 1024 threads and full shared memory on both systems though.
I have also restructured the code according to suggestion 2, but haven't tested if make install works yet.

Thanks !
There's a small issue with the file vkFFT/vkFFT_PlanManagement/vkFFT_Plans/vkFFT_Plan_FFT.H - could you change the extension to a lowercase .h ? I guess windows is case-insensitive but it won't work under linux.
Otherwise it compiles fine, I'll launch the tests.

Oh, sorry, my bad, Visual Studio has renamed this file for some reason. It should be fixed now.

Ok, the radix test passed on the gfx900 AMD card, but it failed for non-radix sizes, but for slightly larger sizes - starting at 3999, 4369, etc..
See:
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-01-amdgfx900/pyvkfft-test.html
I need to reboot the server with that card - AMD driver crashed again.. I can send the kernel codes if that helps
Test is running fine so far on the A40 and V100.

Here is the example code (c2c two sizes and r2c, all out-of-place):
code.zip

In some Rader cases, there were no checks for max threads needed in comparison to how many threads device has - for example 17*257 would need 257 threads as the scaler that makes each thread do multiple FFTs doesn't work with Rader's subprime now.
I added the check and it will split the dispatch in two or disable Rader's algorithm in other cases.
Thank you for pointing this out!

All tests have passed so far (some memory/timeout errors but apparently due to excessive parallel process):
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-04-a40cl/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-04-a40cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-04-v100cl/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-04-v100cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-04-a100cl/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-04-a100cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-04-gtx1080ti_cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-04-amdgfx900/pyvkfft-test.html
The gfx900 test stopped after the first series of non-radix tests (not sure why), I have re-started it (should finish tonight):
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-05-amdgfx900/pyvkfft-test.html
It's starting to look really good. Do you plan merging the develop branch and make a new release soon ?

Yes, I believe the release should be out soon (and it aligns perfectly with the grace period given after the creation of develop branch). I am still not sure how to approach the new autotuning options directly from the C, but this can be done after the release.

Looks like the inverse R2C non-radix, out-of-place (not yet ran the inplace) transforms still have some issues (but probably only one one value, as the N2 norm looks OK but the N_inf is not), for sizes like 2890, 2926, 3060 on the AMD gfx-900:
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-05-amdgfx900/pyvkfft-test.html

There was a missing check in an optimization where the C2R data pre-processing could read directly to registers instead of doing a shared memory read/write and it didn't check if the number of participating threads changes for the first radix stage. I have added the check, so these systems should work now.

There's some issue with this last change - I get a floating point exception when trying an R2C inplace transform of size 34, e.g.:
from pyvkfft.opencl import VkFFTApp
import pyopencl as cl
import numpy as np
ctx = cl.create_some_context()
queue = cl.CommandQueue(ctx)
n= 34
app = VkFFTApp((n,), np.float32, queue, ndim=1, inplace=False, r2c=True, keepShaderCode=1)

import pyopencl.array as cla
a = cla.to_device(queue, np.random.uniform(0, 1, n).astype(np.float32))
app.fft(a, cla.empty(queue, n//2+1,dtype=np.complex64))

This seems to fail on all nVidia cards and also the AMD. But the Apple M1 runs fine..

Oh my bad, the check was a little bit faulty. I have hopefully fixed that.

This looks rather good (some tests are still ongoing), now also with an Apple M1:
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-06-a100cl/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-06-a100cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-06-a40cl/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-06-a40cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-06-v100cl/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-06-v100cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-06-gtx1080ti_cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-06-amdgfx900/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-06-apple-m1/pyvkfft-test.html
Let me know when you can merge and tag this for a new release - I'd like to make a new pyvkfft release soon.

I am still not sure how to approach the new autotuning options directly from the C, but this can be done after the release.

I tend to think these king of functionality are best handled in a packaging layer - like pyvkfft - clearly separating the features. I would not want to use the auto-tuning in a C API, if only because I need to keep control on the allocations going on.

these king of functionality are best handled in a packaging layer
There are many people who use C API directly, and what I was trying to say is that they would also like to have access to improved performance through kernel tuning. And I am not sure yet how to provide them with such functionality (also because of the reasons you mentioned, like allocations control)
As for the release, I will do the limited tests with all the APIs and then do a release in a week if all goes smooth.

It seems I found a very small corner case not working: on the AMD gfx900, the float64 DCT4 fails for 2D transforms and sizes (255, 255) and (799,799), as well as 3D size (255, 255, 255)...
See http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-06-amdgfx900/pyvkfft-test.html
That's really a corner case - I don't put a lot of importance on DCT so not a priority.
Incidentally I'm having a lot of trouble finishing that test suite on the gfx900 card. Somehow once the testsuite reaches the non-radix tests, the card never runs for more than 2-3hours before completely crashing the GPU (rocm-smi crashes) and needing a reboot... Maybe issues with the driver version.

@vincefn I couldn't reproduce the DCT errors with the same setup I used for R2C errors. May I ask you once again to send the generated code, so I verify that we run the same program?
As for the crashes, I also have no idea what could have happened. OpenCL has not been a priority for vendors for quite a while, so there is a non-zero chance of driver issues appearing.

Attached the code for the 255x255 DCT4 float64. Interestingly, the actual calculation seems to be hanged... Haven't tried the others.
dct4_255_255.zip

Yes, the exact same code works on Radeon Pro VII on Windows. Weird.
I guess this version is ready to be merged, I will run some backends checks on it now.

@vincefn I had some time to refactor the code a bit more and added the support for the arbitrary number of dimensions by defining VKFFT_MAX_FFT_DIMENSIONS. Now it is possible to fully mimic FFTW guru interface (apart from the innermost batched R2C/C2R, which I guess can only be done out of place). The implementation should be compatible with all previous versions (I just replaced all [3] arrays and so with [VKFFT_MAX_FFT_DIMENSIONS] ones in the interface), but it can be good to rerun the tests again to be sure.
I will need to update the documentation and then do a full release.
Best regards,
Dmitrii

Looks very interesting ! I'll try to re-run the tests (at a conference next week then vacations..).
I probably won't try to use the arbitrary number of dimensions for the next pyvkfft release - it will need some work and additional unit tests.
Cheers,
Vincent

Ok, I tried some quick tests, but it generates a bunch of errors (C2C, DCT, etc..).
Might be some simple changes are needed, can you take a look ? I don't know if you can also run the pyvkfft test on your side - this just the default basic test on a few dimensions.
Log attached:
logtest.txt

Dear @vincefn,
I think, I found the problem. Starting with this update, specifying configuration.size[1] as an alternative to the configuration.numberBatches with configuration.FFTdim==1 no longer works due to the loop indexing for kernel launch size being tied to the specified configuration.FFTdim. I am not sure if it is a good idea of having a workaround just for this as the number of dimensions is now a compiler constant. Is this the case in pyvkfft tests codes? If so, can you change the initialization of these systems to be done with configuration.numberBatches?
Best regards,
Dmitrii

Ok - currently pyvkfft's calc_transform_axes will not use n_batch if there are <=3 dimensions. For example:
calc_transform_axes((2,36), ndim=1)
returns:
nx=36, ny=2, nz=1, _batch=1
but it should rather return nx=36, ny=1, nz=1, n_batch=2

Ok, by using n_batch the tests seem to run better, but there are still some issues with F-ordered arrays, I'm getting an VKFFT_ERROR_UNSUPPORTED_FFT_OMIT when trying to transform (1D C2C) an array of shape (ny=2, nx=30) with F-ordering... I have to see if I can change something on my side to make that work.

Besides the F-ordered arrays issues, I get (at least, this is quick test) another issue for standard C-arrays:
pyopencl  C2C    (2,30,30,2) axes=   [-2,-3] ndim=None  complex64 lut=None inplace=1  norm=   1 C   FFT: n2=2.32e-07 ninf=3.05e-07 < 3.48e-06 (0.088) 0 iFFT: n2=2.02e-07 ninf=3.05e-07 < 3.48e-06 (0.088) 0   OK
pyopencl  C2C    (2,30,30,2) axes=   [-2,-3] ndim=None  complex64 lut=None inplace=0  norm=   0 C   FFT: n2=1.02e+00 ninf=1.02e+00 < 3.48e-06 (293600.756) 1 iFFT: n2=2.14e-07 ninf=2.94e-07 < 3.48e-06 (0.085) 1 FAIL

So in that case I have an array of shape (2,30,30,2) which is transformed along the two middle dimensions - the forward transform seems to fail when norm=0 (?), but other transforms work (ifft for norm=0, and both fft and ifft for norm=0 and 1). Strange.

Great, the quick test with all the dimensions/strides/axes/norms passes.
I'll launch the complete test suite later.

Looks good for the A40:
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-28-a40cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-28-a40cl/pyvkfft-test.html
Other tests just launched:
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-28-a100cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-28-a100cl/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-28-v100cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-28-v100cl/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-28-gtx1080ti_cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-28-amdgfx900/pyvkfft-test.html

Hi @dtolm the tests are looking good, do you want to merge and tag this for a release?

Dear @vincefn,
I have uploaded the final snapshot of the develop branch, I renamed a bunch of things in the code - namely loose internal Vk references, no changes in the configuration struct. I also corrected some warnings, but there were no structural changes. I have verified that the code builds and works for all backends and will merge it to the main branch tomorrow - so if you can run the tests once again before then, it would be really good.
Best regards,
Dmitrii

Sure, I will do that before the merge tomorrow.

Here goes:
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-31-a40cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-31-a40cl/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-31-a100cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-31-a100cl/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-31-v100cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-31-v100cl/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-31-gtx1080ti_cu/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-31-amdgfx900/pyvkfft-test.html
http://ftp.esrf.fr/pub/scisoft/PyNX/pyvkfft-test/pyvkfft-test-2023-07-31-apple-m1/pyvkfft-test.html

Hi@DTolm - most tests are finished and the others seem to be on a good way towards success
I just have a single pycuda memory error for the v100 and the gtx 1080 which I don't fully understand, but that is on my side.
Even better news, I managed to add support for the arbitrary number of dimensions (see https://github.com/vincefn/pyvkfft/tree/max_fft_dims branch) :-) ! The generic multi-dimensional test now includes up to 5 dimensions for c2c, with all possible permutations for the non-transformed axes (it takes a little while...).
I have not extended the systematic tests in the suite as I don't think it would be useful (I assume that transforms with more than 3 axes rely on exactly the same code as for the 3rd axis, but let me know if I'm wrong).
I've set the default for VKFFT_MAX_FFT_DIMENSIONS to 8 for pyvkfft, configurable when installing.

I have not extended the systematic tests in the suite as I don't think it would be useful (I assume that transforms with more than 3 axes rely on exactly the same code as for the 3rd axis, but let me know if I'm wrong).

Yes, this is correct - there are no more hardforced 3s anywhere and all loop indexes are dependent on VKFFT_MAX_FFT_DIMENSIONS. So a small number of tests should be sufficient.
Configurable VKFFT_MAX_FFT_DIMENSIONS is mostly for innermost batching, but maybe someone will find it useful.
I will update the version number and merge the code later today. Big thanks for helping in refining it!

Ok I am still not that proficient with github, so I am not sure why it closed this issue automatically. But the changes have been merged to the master branch, although the process went not as smoothly as I would want to.
Best regards,
Dmitrii

Apparently it's the title of your commit https://github.com/DTolm/VkFFT/commit/cc410b1b50f5de187b49aac265ce7a799a88d9b7 with a title which says that it fixes this (#25) which automatically triggered the close when you merged (https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue).
It can be surprising at times but in this case it seems to be the right moment !
Cheers,


DO NOT MERGE : Personalizer : Address API review board comments

Latest improvements:
MSFT employees can try out our new experience at OpenAPI Hub  - one location for using our validation tools and finding your workflow.

Contribution checklist:

[x] I have reviewed the documentation for the workflow.
[ ] Validation tools were run on swagger spec(s) and have all been fixed in this PR.
[ ] The OpenAPI Hub was used for checking validation status and next steps.

azure-sdk-for-net PR -  https://github.com/Azure/azure-sdk-for-net/pull/6149

Can one of the admins verify this patch?

Automation for azure-sdk-for-js

Automation for azure-sdk-for-python

Automation for azure-sdk-for-ruby

Automation for azure-sdk-for-go

Automation for azure-sdk-for-net
A PR has been created for you:
https://github.com/azure/azure-sdk-for-net/pull/6142
.NET SDK Commits:
https://github.com/adxsdknet/azure-sdk-for-net/pull/6142/commits/b6773748e0c30c76beff38201c206a593b7542c9

startbuild

Automation for azure-sdk-for-java

@dwaijam  Kindly resolve the merge conflicts

dataplane APIs, not ARM review needed @praries880  -FYI  since ARM does not need to review data plane APIs, "WaitForARMFeedback" is not required.

@dwaijam  kindly take care of the merge conflicts. Also, do you have a rough timeline by when the PR will be ready to merge?

@dwaijam Please resolve all conflicts first. In this way, CI can work again.

@dwaijam  are you  still on this PR? can you pls resolve conflict? Or close the PR and reopen it when you're ready for reviewing?

@dwijam can we close the PR for now since  the PR is over 4 months without updating, there're lots of conflict with latest code base already. Pls create new PR when you're ready for review and merge.

close it for now since the PR is quite old, and no response from author either on github or from email.


idea: support for condensed display of multiple plans?

This is total pie in the sky, I'm just spitballing.
I suspect many folks use a matrix to run multiple terraform plans concurrently (to different environments or cloud accounts or whatever) then feed each plan through a comment action to get a comment per matrix element in the PR.  So if you've got, say, 8 environments, then you get 8 plan comments, which can add excessive scrolling to the PR.
What would be super sweet would be for a way to combine all those plans into a single uber-comment that showed a one line summary of all the changes across all the plans, making it easy for a reviewer to see if a single plan is out of whack versus all the rest, then allowing drill down to a specific plan.
Gitlab does something like this, although not in a comment (they have native TF support in a widget) - I'm struggling to find an actual picture of it in use (I no longer use gitlab), but their design ideas are visible here - https://gitlab.com/gitlab-org/gitlab/-/issues/222964
Obviously there are constraints, particularly around total comment size, and how to handle updating reruns and whatnot, so it wouldn't be trivial.

Interesting thought! I am not entirely sure how to implement the "drill-down" in a single plan since you need to persist the plan somewhere. One option would be to add a bunch of collapsed comments to the PR which contain the full plan and add a link to these collapsed comments. I'll have to think a bit more about other options :eyes:

Example

At the moment it's the calling action's responsibility to ensure the plan is available in the local filesystem before calling terraform-plan-comment?  So I was naively assuming that would contInue - instead of this
token: ${{ github.token }}
planfile: planfile
working-directory: some_dir
id: some_id

then the caller would have to ensure all the plans were available in the local file system, then call uber-terraform-plan-comment with something like:
token: ${{ github.token }}
jobs:
   - id: job1_id
      planfile: job1_planfile
      working-directory: job1_dir
   - id: job2_id
      planfile: job2_planfile
      working-directory: job2_dir

then uber-terraform-plan-comment would have all the plan files available, and then could iterate over the jobs to make the uber comment?

@simonblake-mp Looking at the GitLab issue linked, I really like their proposal diagram, particularly this one:
View GitLab's proposal diagram for Terraform report widget.

Like yourself, I too run Terraform in matrix strategy for multiple different directories in various configurations, and I relate to noisy PRs with a whole bunch of comments. With that said, what do you think about this view?

The list in this screenshout is a product of TF-via-PR GitHub Action and integrated with GitHub's native status checks, enabling you to easily scan the through multiple plan summaries in one place.
It also includes support for commenting TF plan in PR comments and job summaries, as shown here.
View TF-via-PR's output for plans with matrix strategy.

You can probably tell I'm the Action's author, which is why I'm on the look-out for use-cases/feature-requests that users are seeking from Actions like these.

@RDhar sorry for the delay!   That's super pretty - putting the plan/apply output directly into status check line - in a lot of circumstances that would remove the need to post plan comments to the PR at all.

@simonblake-mp Agreed! So if the PR comment is disabled to reduce clutter in the conversation thread, a copy of the plan is attached to the workflow summary for ease of reference/audit (also accessible by clicking Details on the status check line).

putting the plan/apply output directly into status check line - in a lot of circumstances that would remove the need to post plan comments to the PR at all.

Up until few weeks ago, I didn't know how easy this would be :smile: so, definitely possible to write a condensed summary to the status check and put the details somewhere else. I'd be grateful for a PR (following some more concrete proposal) unless it is fine that this feature will still take some time :smile:


test(annotation): cover redistribute

In this pull request, the function redistribute(...) from the /distribution/main.py module is covered. The tests for this function can be found in the file /tests/test_distribution.py

The pr checks didn't pass, but before I fix them, I would like to see how we could simplify the patch for delete_tasks

I merged the contents of this pr with the pr 916, so this one is no longer relevant and ill close it


add aar package support

@DonnieWest @artur-shaik aar package dependency has been supported

it is very strange,before this PR,also can import V7 package,but can not import classes in this package.after this PR also can not

no need this PR mvn will unpacked arr in to /target/unpacked-libs/ and auto add all jars into build-classpath


angular/protractor

So i am new to cucumber. My tests results are shown before it executes. It Shows all passed even though the browser is still loading.
Below is my *page.js
var CalculatorPage = function() {
//const {setDefaultTimeout} = require('cucumber');
this.get = function() {
browser.get('');
};
Below is my steps.js file
this.Given(/^The LoginPage is open$/, function () {
this.page.get();
});

Hi there!
Your question is better suited for StackOverflow or Gitter. Please ask a question there with the 'protractor' tag or post in the Gitter Channel to get help.
From the the getting help section of the README:

Please ask usage and debugging questions on StackOverflow (use the "protractor" tag) or in the Angular discussion group. (Please do not ask support questions here on Github.)

Thanks!


OnStartLocalPlayer() hook of NetworkBehavior doesn't work but OnStartClient does

OnStartLocalPlayer() hook of NetworkBehavior doesn't work but OnStartClient does.
Just use OnStartLocalPlayer() to log something. After connecting to the server and seeing the localPlayer, it doesn't fire but OnStartClient() works. What's up with that?
Expected behavior
For OnStartLocalPlayer() to fire.

OS: Windows
Build target: Windows
Unity version: 2021 LTS
Mirror branch: 66.0.9

Are you missing the override wording?
public override void OnStartLocalPlayer()
        {
            print("OnStartLocalPlayer");
        }

https://mirror-networking.gitbook.io/docs/community-guides/quick-start-guide
Remember to refer to the guides
and  mirror/examples
and  ScriptTemplates  folder  :)

I am using indeed:
public override void OnStartLocalPlayer()
    {}

Indeed I think you are right it's because the prefab is not registered as a Player Prefab in NetworkManager. The player prefab is a prefab registered in the Registered Spawnable Prefabs.

@JesusLuvsYooh what would you recommend to use? What are the ways to detect the appearance of the local player GameObject? In my set I have two prefabs that are chosen for a player, Male or Female prefabs.


java.lang.NoClassDefFoundError: Failed resolution of: *****/R$raw;

when I use gradle setting like this:
gradle-wrapper: distributionUrl=https://services.gradle.org/distributions/gradle-3.3-all.zip
classpath 'com.android.tools.build:gradle:2.3.3'
I run application get a crash.
so I try to use gradle setting like this:
gradle-wrapper: distributionUrl=https://services.gradle.org/distributions/gradle-2.14.1-all.zip
classpath 'com.android.tools.build:gradle:2.2.3'
then the application works properly
sorry,my english is poor

i have the same issues.

java.lang.NoClassDefFoundError: Failed resolution of: Lcom/inveno/commonlib/R$layout;
I have the same issues,my android studio version is 3.0, classpath 'com.android.tools.build:gradle:2.3.3' ,gradle-wrapper: distributionUrl=https://services.gradle.org/distributions/gradle-3.3-all.zip,
how to do to solve this issuues?

java.lang.NoClassDefFoundError: Failed resolution of: Lcom/inveno/commonlib/R$layout; I have the same issues

java.lang.NoClassDefFoundError: Failed resolution of: Lcom/laba/test/lintwo/R$layout; I have the same issue

it seems that the task embedRClass cannot product jar of new generated R*.java to bundle directory.
task embedRClass(type: org.gradle.jvm.tasks.Jar, dependsOn: collectRClass) {
doLast{
println "EMBED R CLASS"
destinationDir file("$bundle_release_dir/libs/")
from base_r2x_dir
}
}

@tommy8421 Do you know how to solve this problem now?

@Whoisurdady please visit to get the fixed version script: https://github.com/tommy8421/fat-aar-fixed

@tommy8421
God ! you save my life !   it works with gradle:2.3.0 !!!


Add argument for missing property repeatTableHeaders

according to the latest Polarion api the exportDocumentToPDF function has 6 arguments:
String paperSize,  String orientation,  boolean fitToPageWidth,  boolean generateBookmarks,  boolean includeHeaderFooter,  boolean repeatTableHeaders
the current implementation was missing the repeatTableHeaders argument. This PR fixes that.

What version of Polarion is that? Because 21r2 does not have repeatTableHeaders.
I was well aware at some point there would be something version specific added, but there is nothing ready for that. I'm willing to accept a PR on this, but this needs to be optionally enabled then. Perhaps a version string in the Polarion class constructor.

@jesper-raemaekers this is on 23.04 we always use the latest version. If there is an api call to request a version we can make it depending on that version.


bug: fix GitHub CLI installation

GitHub CLI installation via apt is currently broken and it seems like it won't be fixed any time soon. Probably best to change the installation method to using GitHub releases directly.

:tada: This issue has been resolved in version 1.11.10 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:


Add 810 & 855 for v4010

@kputnam Thanks for your feedback. I believe I addressed all your concerns.

@irobayna For some reason the CI builds aren't running, but the test suite passed locally. I believe this is ready for release.


Silence missing BPF program error

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area API-version

/area build

/area CI

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

/area libscap-engine-bpf

/area libscap-engine-gvisor

/area libscap-engine-kmod

/area libscap-engine-modern-bpf

/area libscap-engine-nodriver

/area libscap-engine-noop

/area libscap-engine-source-plugin

/area libscap-engine-savefile

/area libscap-engine-udig

/area libscap

/area libpman

/area libsinsp

/area tests

/area proposals

Does this PR require a change in the driver versions?

/version driver-API-version-major

/version driver-API-version-minor

/version driver-API-version-patch

/version driver-SCHEMA-version-major

/version driver-SCHEMA-version-minor

/version driver-SCHEMA-version-patch

What this PR does / why we need it:
PR https://github.com/falcosecurity/libs/pull/943 has introduced ability to exclude a set of tail-called programs from loading. Yet current such excluded program will generate an error message, which could be misleading. There is no logging infrastructure in this module, so remove the error message, then reintroduce it as a debug one when logger is in place.
Which issue(s) this PR fixes:

Fixes #943
Special notes for your reviewer:
Ideally we would like to keep such message, but there is no logger there in libpman. I'm going to work on a follow-up PR to introduce logging capabilities, consistent with other libraries, then reintroduce this message at the debug level.
Does this PR introduce a user-facing change?:

NONE

[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: erthalion
Once this PR has been reviewed and has the lgtm label, please assign mstemm for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

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

Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.
:memo: Please follow instructions in the contributing guide to update your commits with the DCO
Full details of the Developer Certificate of Origin can be found at developercertificate.org.
The list of commits missing DCO signoff:

cd8b55d Silence missing BPF program error

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

Ideally we would like to keep such message, but there is no logger there in libpman. I'm going to work on a follow-up PR to introduce logging capabilities, consistent with other libraries, then reintroduce this message at the debug level.

Maybe we could replace this message when we will have the logging capability in place, in this way, we don't lose this info for an undefined period of time...WDYT? BTW thank you for working on this, a logging framework is something we really need in scap/libpman!
Just a minor note: in theory, we could remove libpman now, if it requires extra work in this logging task. libpman was used as an helper library in the initial modern bpf testing framework but now that we have the v-table and the engine architecture, this library is no longer necessary

Ideally we would like to keep such message, but there is no logger there in libpman. I'm going to work on a follow-up PR to introduce logging capabilities, consistent with other libraries, then reintroduce this message at the debug level.

Maybe we could replace this message when we will have the logging capability in place, in this way, we don't lose this info for an undefined period of time...WDYT? BTW thank you for working on this, a logging framework is something we really need in scap/libpman!

My intention here is to optimize the state of things short-term as well. The logging message already produces confusion among users, while bringing value in form of debugging information. It would take some time to introduce a proper logging, and while the downsides are outweighing the advantages, it makes sense to me to turn it off for now. Does it sound reasonable for you, folks?

Just a minor note: in theory, we could remove libpman now, if it requires extra work in this logging task. libpman was used as an helper library in the initial modern bpf testing framework but now that we have the v-table and the engine architecture, this library is no longer necessary

Sounds interesting, but to be honest I haven't followed the recent development so closely, and need to think about this possibility. Generally speaking less moving parts the better of course.

Hey, it seems like someone heard your prayers :laughing: https://github.com/falcosecurity/libs/pull/1395

Hey, it seems like someone heard your prayers 😆 #1395

Prayers is a strong word, but it's nice to see this development :)

I think this can now be improved to log a debug message since #1395 was merged :)

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Provide feedback via https://github.com/falcosecurity/community.
/lifecycle stale

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
Provide feedback via https://github.com/falcosecurity/community.
/lifecycle rotten

/remove-lifecycle rotten
@erthalion news? :)

@erthalion news? :)

Lots of news, which one you would like to hear? :)
Haven't had a chance yet to rework this one, give me a couple of days.

Lots of news, which one you would like to hear? :)

Ahahaha hope they are good news :D

Haven't had a chance yet to rework this one, give me a couple of days.

No problem!

@FedeDP this should look better now. Not sure, is there anything needed to trigger the tests?

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

Needs approval from an approver in each of these files:

OWNERS [FedeDP]

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

LGTM label has been added.  Git tree hash: 4c7cedd301e0f4073123e462fa6d7b3cbf3f286f

/milestone 0.15.0

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

Needs approval from an approver in each of these files:

OWNERS [Andreagit97,FedeDP]

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

Thanks, @FedeDP @Andreagit97 !


Update Settings “general”

Automatically generated by Netlify CMS

👷 Deploy Preview for docbracing processing.

Name
Link

🔨 Latest commit
2d9d4c13a8581ff75b8d73bd052fb5b1a7c8964f

🔍 Latest deploy log
https://app.netlify.com/sites/docbracing/deploys/65107bf5a206f70008d6234d


Levioso and MouseParallax are not working as expected.

Describe the bug
I'm posting this here, as it seems to be related to the Nuxt module.
I've already posted some more info here https://github.com/Tresjs/cientos/issues/369 and here https://github.com/orgs/Tresjs/discussions/642
The problem is that both Levioso and MouseParallax are not working as expected, while other stuff from cientos don't seem to have issues (in my case, 3d text is working).
The problem started with these two components not working only in development environment, but with the latest dependencies updates (probably caused by Nuxt), they don't work in the built production site too. (the problem with development is on a private project that I cannot share at the moment).
The issue with the production environment can be seen also on the linked repository, in the latest commits on the main branch. The latest working configuration is available at the working tag: https://github.com/stefanobartoletti/tresjs-playground/tree/working tag.
A live demo is also available here https://tresjs-playground.vercel.app/3d-text
If more info is needed, please ask :)
Reproduction
https://github.com/stefanobartoletti/tresjs-playground
Steps to reproduce

pnpm i
pnpm generate
pnpm preview

System Info
No response
Used Package Manager
pnpm
Code of Conduct

[X] I agree to follow this project's Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] The provided reproduction is a minimal reproducible example of the bug.

I noticed that even the example available here https://tresjs-playground.vercel.app/rotating-ring is not working as expected, but it is not using Levioso or MouseParallax.
It is using an onLoop function, as you can see from its source here https://github.com/stefanobartoletti/tresjs-playground/blob/main/pages/rotating-ring.vue
I had a look at the source of both Levioso and MouseParallax and I see that they are internally using onLoop too, so maybe the problem is originating there.
I'm not sure, but maybe this is related to #97

A quick report: updating cientos to version 3.9.0 seems to have partially solved this issue, these animations work correctly on the static production site, but I still have some problems in the development environment (they are not working)


Add The Separon from Kerbal Stuff

This pull request was automatically generated by Kerbal Stuff on behalf of TheCardinal, to add The Separon to CKAN.
Please direct questions about this pull request to TheCardinal.

Closed by https://github.com/KSP-CKAN/NetKAN/pull/2466


Unexpected inconsistent behaviour when using 'model_construct'

Initial Checks

[X] I confirm that I'm using Pydantic V2

Description
I came across something unexpected / inconstistent when passing extra values to model_construct.
class A(BaseModel):
    attribute: int

a = A.model_construct(another_attribute=":(")

In the documentation, it says:

[...] Creates a new model setting dict and pydantic_fields_set from trusted or pre-validated data. Default values are respected, but no other validation is performed. Behaves as if Config.extra = 'allow' was set since it adds all passed values

When inspecting the created object, the attribute doesn't seem to be present:
a  # A()
a.model_dump()  # {}
a.model_extra  # None

Then, however, I discovered, that the attribute is indeed present and can (must) be accessed directly as it doesn't show up in the dump result:
a.another_attribute  # ':('

This is somehow unexpected, because I thought the attribute would also appear in the repr and model_dump result.
What I found to be inconsistent is the following sentence:

Behaves as if Config.extra = 'allow' was set since it adds all passed values

When I manually set extra="allow", the extra values to model_construct show up as expected:
class B(BaseModel):
    model_config = ConfigDict(extra="allow")
    attribute: int

b = B.model_construct(another_attribute=":)")

b  # B(another_attribute=':)')
b.another_attribute  # ':)'
b.model_dump()  # {'another_attribute': ':)'}
b.model_extra  # {'another_attribute': ':)'}

But this somehow contradicts the statement, that model_construct just behaves as if "Config.extra = 'allow' was set" as it actually takes into account the current configuration.
A notice of this behavior in the documentation would be nice.
Example Code
from pydantic import BaseModel, ConfigDict

class A(BaseModel):
    model_config = ConfigDict(extra="forbid")  # ignore behaves the same

    attribute: int

a = A.model_construct(another_attribute=":(")

a  # A()
a.another_attribute  # ':('
a.model_dump()  # {}
a.model_extra  # None

class B(BaseModel):
    model_config = ConfigDict(extra="allow")
    attribute: int

b = B.model_construct(another_attribute=":)")

b  # B(another_attribute=':)')
b.another_attribute  # ':)'
b.model_dump()  # {'another_attribute': ':)'}
b.model_extra  # {'another_attribute': ':)'}

Python, Pydantic & OS Version
pydantic version: 2.5.2
        pydantic-core version: 2.14.5
          pydantic-core build: profile=release pgo=false
                 install path: /opt/homebrew/Caskroom/mambaforge/base/envs/test/lib/python3.12/site-packages/pydantic
               python version: 3.12.0 | packaged by conda-forge | (main, Oct  3 2023, 08:36:57) [Clang 15.0.7 ]
                     platform: macOS-14.1.1-arm64-arm-64bit
             related packages: typing_extensions-4.8.0 pydantic-settings-2.1.0

@ThomasMarwitzQC,
Agreed, this is inconsistent! PRs welcome to either clean up the docs on this concept or (better) change the behavior of model_construct to reflect the current expectation set by the docs!

@sydney-runkle I can work on this.

@sydney-runkle
What should be the expected output for the function ?
Should this be same as a class with model_config['extra '] = allow ?
I am able to recreate the same output by calling model_rebuild with model_config['extra'] = 'allow' but then this changes the initial model_config.
cls.model_config['extra'] = 'allow'
cls.model_rebuild(force=True)

@StrawHatDrag0n and @ThomasMarwitzQC,
You both have good points. One other note - the concepts portion of the docs says the following, which is consistent with the current behavior, but not the claim that model_construct always behaves as if Config.extra = 'allow' was set:

For models with model_config['extra'] == 'allow', data not corresponding to fields will be correctly stored in the pydantic_extra dict.

I'm inclined to have us:

Add an update to the docs to make sure we're clearly explaining the behavior of model_construct
Add an extra parameter to the model_construct function to allow more fine tuned customization of the model_construct behavior with extra value arguments.

I think it's valuable for us to try to avoid introducing any breaking changes for users who might be depending on the current behavior of model_construct logic.
A few more details on this extra parameter behavior:

If extra isn't specified in the call to model_construct, I think we should observe the current behavior
If extra='allow' is specified in the call to model_construct, follow the path of behavior taken when extra='allow' on the model
If extra='ignore', I don't think it makes sense for us to store the extra values in the __dict__ of the newly created model. I could also be convinced that it doesn't make sense for us to store these values in __dict__ by default (as is the current behavior), as this doesn't occur with __init__, but this would be a breaking change.
If extra='forbid', follow a pattern similar to that of said case in __init__.

Excited to hear your thoughts on this. Happy to review your PR :). Thanks for your help on this!

@sydney-runkle
Regarding 1., adding an update to the docs sounds like a good idea!
Regarding the proposed extra parameter for model_construct:

It sounds reasonable to not introduce breaking changes.
extra = None (unspecified): this sounds fine
extra = 'ignore': I know to little about the current behaviour in __init__, but I would stick with your initial suggestion that if explicitely setting instance.model_construct(extra='ignore'), we should not add these values to __dict__.
extra = 'forbid': Same as 'ignore'? This case doesn't really make sense as the whole point of model_construct is to not perform validation and 'forbid' somehow implies that an Exception should be raised if the attribute was present.
extra = 'allow': I quickly checked the behaviour if I just include the passed values in _extra during model_construct. This leaves me with the problem that when model_config.extra = ignore | forbid, repr(instance) works as expected, instance.model_extra also works as expected, but instance.model_dump doesn't:

extra = ["allow", "ignore", "forbid"]:

   class Foo(BaseModel, extra=extra):
       x: int

   model = Foo.model_construct(x=1, y=2)
   assert model.x == 1
   assert model.y == 2

   assert model.model_extra == {'y': 2}
   assert 'y=2' in repr(model)
   assert model.model_dump() == {'x': 1, 'y': 2} # This fails for extra='forbid' or 'ignore'

So to fix that, we might need to do sth like @StrawHatDrag0n suggested, but that would interfere with the normal model_config. If there is no easy fix to this, we should either note this inconsistent behaviour or rethink the use of this new extra parameter at all, or maybe change its value range (as forbid already doesn't make sense in these particular circumstances).
Please let me know, if anything I said is unclear to you - I'm happy to elaborate :)

@ThomasMarwitzQC,
All good points. Yeah, I'd agree that an extra='forbid' setting doesn't really make sense in the context of model_construct.
Consider this snippet:
from pydantic import BaseModel

for extra in ["allow", "ignore"]:
    class Foo(BaseModel, extra=extra):
        x: int

    # using __init__
    f = Foo(x=1, y=2)

    print(extra)
    print(f"__dict__: {f.__dict__}")
    print(f"model_extra: {f.model_extra}")
    print(f"model_dump: {f.model_dump()}")
    print("\n")

"""
allow
__dict__: {'x': 1}
model_extra: {'y': 2}
model_dump: {'x': 1, 'y': 2}

ignore
__dict__: {'x': 1}
model_extra: None
model_dump: {'x': 1}
"""

for extra in ["allow", "ignore"]:
    class Foo(BaseModel, extra=extra):
        x: int

    # using model_construct
    f = Foo.model_construct(x=1, y=2)

    print(extra)
    print(f"__dict__: {f.__dict__}")
    print(f"model_extra: {f.model_extra}")
    print(f"model_dump: {f.model_dump()}")
    print("\n")

"""
allow
__dict__: {'x': 1}
model_extra: {'y': 2}
model_dump: {'x': 1, 'y': 2}

ignore
__dict__: {'x': 1, 'y': 2}  # <-- y is not ignored, I think we should change this
model_extra: None
model_dump: {'x': 1}
"""

I'd agree that if we use the new extra param, we should narrow its scope of allowed values. ignore and allow should be fine.
I'm currently leaning:

Default to model's extra setting
If ignore, we should model the behavior of __init__, ie not adding the extra values to the __dict__
If allow, we can keep the current behavior on said branch, which models the __init__ behavior

I think it's important to note in the docs that the default setting for a model's config.extra is ignore, so if extra is not set on the model config nor on the call to model_construct, then ignore would be used here. Though this does seem a bit counterintuitive to me because of the nature of the model_construct method, this is sort of the current behavior.
One thing I haven't considered - what if the model's extra setting is forbid?

To be honest, the naming is already pretty confusing for me. How about renaming the attribute to behaviour (or something like behaviour_extra) with values old, keep, discard (and potentially more / other options).
For the keep option however, we currently cannot over a really consistent behaviour (even if the modification I suggested above is implemented):

On the one hand, additional attributes are shown in repr() and model_extra
On the other hand, additional attributes are not shown in model_dump.

@sydney-runkle I guess the model_dump behaviour is written in Rust? At least I couldn't find the Python implementation. Would it be possible to change its behaviour s.t. attributes contained in model_extra are serialized even when config.extra = forbid | ignore or would that be a breaking change?

@ThomasMarwitzQC,

@sydney-runkle I guess the model_dump behaviour is written in Rust? At least I couldn't find the Python implementation. Would it be possible to change its behaviour s.t. attributes contained in model_extra are serialized even when config.extra = forbid | ignore or would that be a breaking change?

This would be a breaking change, and isn't one that we want to implement. You might want to look into the SerializeAsAny annotation for behavior like this.
Re your other questions - I'll chat with our team on Monday morning and get back to you then 👍.

@ThomasMarwitzQC,
Sorry for delay in responding here. I'm planning on opening a PR early next week with a docs update / slight change in the behavior of model_construct. Stay tuned for a fix!
Thanks again for bringing this to our attention.

@sydney-runkle Thank you so much :)

@ThomasMarwitzQC,
Apologies for the delay - finally got around to this PR :).


Can't run the server

I don't know whether I am doing anything wrong, but when I try to run this line:
.\streammachine-cmd --config ./config/standalone.json
But the only thing happens is to open the notepad with the content of "streammachine-cmd" file.
Am I missing something ? The documentation is not so clear about that.

I presume you are on Windows. On Windows you can't use cmd files from main directory, they are runnable for linux only. Try this:
node streammachine-cmd --config ./config/standalone.json

Worked for me on Windows, thanks.

@davidrock thanks for confirmation. Closing this.


Error in performing DA Analysis

I want to obtain differentially expressed genes over samples of patients infected with a disease along with control samples. I have used the following command:
Rscript CAMPP.R -d data2.xlsx -m metadata2.xlsx -v array -g Accession,Code -t log2 -l 0.0 -s TRUE -f 0.6,0.05 -n DEG
However, this returns the following error:
Error: Aesthetics must be either length 1 or the same as the data (32): fill and colour
Backtrace:
█

└─ggplot2::ggsave(...)
├─grid::grid.draw(plot)
└─ggplot2:::grid.draw.ggplot(plot)

├─base::print(x)

└─ggplot2:::print.ggplot(x)

  ├─ggplot2::ggplot_build(x)

  └─ggplot2:::ggplot_build.ggplot(x)

    └─ggplot2:::by_layer(function(l, d) l$compute_aesthetics(d, plot))

      └─ggplot2:::f(l = layers[[i]], d = data[[i]])

        └─l$compute_aesthetics(d, plot)

          └─ggplot2:::f(..., self = self)

            └─ggplot2:::check_aesthetics(evaled, n)

Execution halted
Do I need some modifications in my data for this command to work?

Hi @SrijaniBagchi :-)
Okay so from what you write, I have a couple of ideas as to what could be the issue:

Remove the flag -l, as it specifies lasso regression. Even if you do want that to perform this analysis in addition to your DEA, the parameter cannot be set to 0.0 (it should be between 0.1-1.0), as this is essentially ridge regression, which CAMPP does not support. This is stated somewhere in the way to long manual I hope ;-) So this instead:

Rscript CAMPP.R -d data2.xlsx -m metadata2.xlsx -v array -g Accession,Code -t log2 -s TRUE -f 0.6,0.05 -n DEG

The name of your id variable in the above is = Accession, I am just checking that you are giving it a sample ID and not a gene/protein id?
Also the framework which makes the DEA contrasts and feeds it to limma, does not do well with groups (here your Code) which consists only of integers/numbers. So, if this is the case you should add a letter to the group names to make sure that it is understood as a string. Also, the group names cannot have a - in them, as this character is used as the symbol for contrast in them design matrix.
Make sure that Code has atleast two groups with 5 samples in each group.

Make sure your matrices both data en metadata look like the files found here: https://github.com/ELELAB/N-glycan-TIF/tree/master/Data/DataExamples. You can see an example of how the data need to be set up, you can even download and test the pipeline with these data.

Check that your R version in >= 4.0.0

If non of the suggestions above fix your problem, let me know, and I will have a deeper look into it when I have the time.
Cheers,
Thilde

Hey @ThildeBT ,
I have followed your instructions but the same error persists. Here is what my datafiles look like:
data2.xlsx
metadata2.xlsx
I am using R-version 4.0.0(2020-04-24) and altered by command to:
Rscript CAMPP.R -d data2.xlsx -m metadata2.xlsx -v array -g Accession,Code -t log2 -s TRUE -f 0.6,0.05 -n DEG
It would be a great help if you could take a look at it.
Thanks and Regards,
Srijani

Hey @SrijaniBagchi,
Okay I have figured it out. You had the sample ids with all capital letters in the data file and with a small in the metadatafile. They need to match. However I have now corrected the code so the paring is not case dependent as this is of course more robust. I have also corrected the code a little so in another way, so i suggest you to re-download the repository or rater the scripts.
Here are your datafiles back, corrected also for the / in the gene names as this may be problem you would otherwise run into.
data2.xlsx
metadata2.xlsx
As soon as you have downloaded them I suggest you to remove them from your comment above, and I will do the same, so they are note visible here to others :-)
On a different note, I noticed that one of your sample appears to be an exteme outlier from the MDS plot.
Cheers,
Thilde

Tanks you so much @ThildeBT !
Sorry for bothering you with such a small issue

@SrijaniBagchi, no worries at all, hope you get it to work now :-) I will leave the thread open for tomorrow if for some some other reason it still throws an error.
Cheers,


[CI] Test against Ruby 2.5

Coverage decreased (-0.04%) to 90.116% when pulling b43f455255db6d2175d26217c74394c11aa72ab0 on nicolasleger:patch-1 into 8a63a0c850a3a183064df801c5f9136da24b6ee0 on matschaffer:master.

Sweet, thanks @nicolasleger!


⚠ Samsung has degraded performance

In 538f196, Samsung (https://samsung.com) experienced degraded performance:

HTTP code: 200
Response time: 2399 ms

Resolved: Samsung performance has improved in 147da12.


allow email rule to handle array of values - EXAMPLE

Would be possible showing an example of how to use the array of value in the email field?
I tried to use a comma separated string as input, and transformed it in an array just before calling the validator, but without luck.
Probably I misinterpreted the goal of the rule, and I would be grateful if someone could explain it to me.
Thanks

I will try to implement a multiple email field support soon, currently if
you pass an array of emails it should be handled correctly. But not comma
separated values, will fix it soon since it is the official usage.

I will try to look into it too:))
Sent from my iPhone

On May 10, 2018, at 6:44 PM, Abdelrahman Awad<EMAIL_ADDRESS>wrote:
I will try to implement a multiple email field support soon, currently if
you pass an array of emails it should be handled correctly. But not comma
separated values, will fix it soon since it is the official usage.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.


Update changelog for v2

@connorferster @ccaprani @normanrichardson @aegis1980 @Agent6-6-6 @Spectre5
Attempted to update the changelog to capture changes from v1.0.8 to v2.0.0 - please let me know if I've missed out anything important or if you have any suggestions!

Did a quick review. Will do a larger review this week.


simple-uploader first chunk number is 1

The index of the chunk in the current upload. First chunk is 1 (no base-0 counting here).

README.md#How do I set it up with my server?

Thank you

@pionl This needs to be reverted.  There is a missing use Illuminate\Http\Request at the top.  This is resolved in my pr at https://github.com/pionl/laravel-chunk-upload/pull/56 which also fixes another issue.

FYI: Should be resolved in my merge: https://github.com/pionl/laravel-chunk-upload/commit/db6b455054556723a311b017219d159d50e4a74d


Dynamic logzio type based on record fields

I would like to set the logzio type dynamically, based on a property in the record.
I tried something like that:
endpoint_url "${#{ENV['LOGZIO_URL']}?token=#{ENV['LOGZIO_TOKEN']}&type=#{record['custom_type']}"

But that failed:
undefined local variable or method `record' for £<Binding:0x00007f02fc64f770

Can you point me in the right direction?

@omerlh
Hi Omer. Are you trying to send each log with a different type or you want all the logs to have the same type?

I want to have different types for different services, based on a parameter the service sends in the log record.

@omerlh
You can add a field to the message.
https://docs.fluentd.org/v0.12/articles/filter-modify-apache

And remove the type from the URL?

Yep :)

Nice! I can confirm it's working :)


Added auto-comment app to the repo

Added my new Probot app to the list.

Checklist

[x] If you're adding a listing for your app, be sure your app is in the /_apps/ folder before opening this Pull Request.
[x] All comments in frontmatter need to be removed.

Not really sure whats up with the CI?

Thanks again @hiimbex .
I have made the changes and removed the HTML from the markdown 👍


If no file is passed to the cmd-line app, read from stdin

Usage:
$ genderbias -f my-file.txt

or
$ cat my-file.txt | generbias

One of the biggest applications I anticipate here is the ability to use the
system without dropping the data to disk: this can save us some roundtrip
time on the webapp, and allow use in common command line text processing
tools.
For example, i used
wget | pandoc | genderbias | grep
to search for our souces of bias on a webpage
Super dorky, for sure
On Sun, May 20, 2018, 1:35 AM Neil Pilgrim<EMAIL_ADDRESS>wrote:

@neiljp approved this pull request.
@j6k4m8 https://github.com/j6k4m8 This feature can be useful in some
situations, I am sure. Do you have an example?
I'm aware that pipes are simpler, but I realized that this is also
achievable, due to the Document implementation, via:
genderbias -f "cat example_letters/letterofRecW"
Do you think we should avoid allowing that, or just leave it as a strange
feature? :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/molliem/gender-bias/pull/32#pullrequestreview-121636400,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAqVB5iLHBrmakEYpRbvY8lhPY4E9TZVks5t0QCGgaJpZM4UF7fU
.