KeyError: 'transifex_project_name' when updating .tx/config

When I was updating the txconfig using
$ sphinx-intl create-txconfig
$ sphinx-intl update-txconfig-resources --transifex-project-name python-35-tw
Traceback (most recent call last):
  ...
  File "/Users/liang/.virtualenvs/pydoc/lib/python3.5/site-packages/sphinx_intl/commands.py", line 536, in update_txconfig_resources
    args = [arg % locals() for arg in args_tmpl]
  File "/Users/liang/.virtualenvs/pydoc/lib/python3.5/site-packages/sphinx_intl/commands.py", line 536, in <listcomp>
    args = [arg % locals() for arg in args_tmpl]
KeyError: 'transifex_project_name'

It failed with KeyError, which the argument should be passed to the function.
A quick survey showed it might be related with using locals() inside list comprehension,
# sphinx_intl/commands.py:536
args = [arg % locals() for arg in args_tmpl]

If I change to
d = locals()
args = [arg % d for arg in args_tmpl]

then the code works.
A demo shows this different behavior in Python 2 and 3.
def print_dict(d):
    print(d)

def main():
    a, b = 3, 4
    print_dict(locals())
    [print_dict(locals()) for i in range(2)]

if __name__ == '__main__':
    main()

# python 2.7
# {'a': 3, 'b': 4}
# {'a': 3, 'i': 0, 'b': 4}
# {'a': 3, 'i': 1, 'b': 4}

# python 3.4 / 3.5
# {'a': 3, 'b': 4}
# {'.0': <range_iterator object at 0x1029851e0>, 'i': 0}
# {'.0': <range_iterator object at 0x1029851e0>, 'i': 1}

My Python packages versions:
Sphinx==1.3.3
sphinx-intl==0.9.7
transifex-client==0.11

Your investigation helps me fixing the issue. Thanks!


Execute by cucumber tag issue

cucumber tags will have multiple comma separated values and escape the commas.

tags with comma separated values are allowed.


Bug loading some cif files from pdb

Hey there,
I found an issue when opening some .cif files from pdb.
I initially found this in my app but was able to replicate in your demo viewer. If for example you get this file: https://www.ebi.ac.uk/pdbe/static/entry/10mh_updated.cif
and try to open it in http://nglviewer.org/ngl/ it doesn't open and the browser console shows:
THREE.WebGLRenderer 95
ngl.js:1 STAGE LOG loading file '10mh_updated.cif'
ngl.js:1 STAGE LOG error loading file: 'TypeError: Cannot read property 'push' of undefined'
nglviewer.org/:1 Uncaught (in promise) error loading file: 'TypeError: Cannot read property 'push' of undefined'

I have similar issues with other structures: 2bp5 6owo
But then there are others such 101m that work OK.
I have partially solved my issue by opening the sequence from a different source. But thought of reporting this here in case there is an issue with the library. If you think is something wrong with the .cif file, just let me know and I'll report this to pdb.
Cheers,
Gustavo

I think the problem is in this block
loop_
_citation_author.citation_id
_citation_author.name
_citation_author.ordinal
primary 'Sheikhnejad, G.' 1
primary 'Brank, A.' 2
primary 'Christman, J.K.' 3
primary 'Goddard, A.' 4
primary 'Alvarez, E.' 5
primary 'Ford Jr., H.' 6
primary 'Marquez, V.E.' 7
primary 'Marasco, C.J.' 8
primary 'Sufrin, J.R.' 9
primary
;O'gara, M.
; 10
primary 'Cheng, X.' 11`

There's a name with a quote in it and it's being rendered over multiple lines - a quick look at the standard (https://www.iucr.org/resources/cif/spec/version1.1/cifsyntax) suggests this is correct, and NGL is parsing it wrongly.
(search for "The special sequence of end-of-line followed immediately by a semicolon" in above link)
Anyone fancy trying to fix this?


GLES2 - shader compiler fails with multiple macro redefinition's error

Godot version:
Godot Engine v3.2.2.beta.custom_build.16ec167b7
OS/device including version:
Android 4.4, OpenGL ES 2.0 PowerVR SGX 540
GLES: Imagination Technologies, PowerVR SGX 540, OpenGL ES 2.0
GL_EXT_debug_marker GL_OES_rgb8_rgba8 GL_OES_depth24 GL_OES_vertex_half_float GL_OES_texture_float GL_OES_texture_half_float GL_OES_element_index_uint GL_OES_mapbuffer GL_OES_fragment_precision_high GL_OES_compressed_ETC1_RGB8_texture GL_OES_EGL_image GL_OES_EGL_image_external GL_OES_required_internalformat GL_OES_depth_texture GL_OES_get_program_binary GL_OES_packed_depth_stencil GL_OES_standard_derivatives GL_OES_vertex_array_object GL_OES_egl_sync GL_OES_surfaceless_context GL_EXT_discard_framebuffer GL_EXT_blend_minmax GL_EXT_multi_draw_arrays GL_EXT_multisampled_render_to_texture GL_EXT_shader_texture_lod GL_EXT_texture_format_BGRA8888 GL_EXT_texture_rg GL_IMG_shader_binary GL_IMG_texture_compression_pvrtc GL_IMG_texture_npot GL_IMG_texture_format_BGRA8888 GL_IMG_read_format GL_IMG_program_binary GL_IMG_uniform_buffer_object GL_IMG_multisampled_render_to_texture GL_KHR_debug
EGL
EGL_IMG_client_api_ogl EGL_KHR_image EGL_KHR_image_base EGL_KHR_gl_texture_2D_image EGL_KHR_gl_texture_cubemap_image EGL_KHR_gl_renderbuffer_image EGL_KHR_vg_parent_image EGL_IMG_cl_image EGL_ANDROID_blob_cache EGL_ANDROID_image_native_buffer EGL_ANDROID_framebuffer_target EGL_ANDROID_native_fence_sync EGL_ANDROID_recordable EGL_KHR_fence_sync EGL_IMG_context_priority EGL_IMG_hibernate_process EGL_IMG_image_plane_attribs EGL_KHR_surfaceless_context EGL_KHR_wait_sync EGL_KHR_create_context

Issue description:
shader compilation fails with
**ERROR**: GL ERROR: Source: Shader Compiler	Type: Error	ID: 0	Severity: High	Message: ERROR: 0:6: Syntax error, 'ENABLE_TANGENT_INTERP' macro redefinition
   At: drivers\gles2\rasterizer_gles2.cpp:133:_gl_debug_print() - GL ERROR: Source: Shader Compiler	Type: Error	ID: 0	Severity: High	Message: ERROR: 0:6: Syntax error, 'ENABLE_TANGENT_INTERP' macro redefinition
**ERROR**: GL ERROR: Source: Shader Compiler	Type: Error	ID: 0	Severity: High	Message: ERROR: 0:8: Syntax error, 'LIGHT_USE_RIM' macro redefinition
   At: drivers\gles2\rasterizer_gles2.cpp:133:_gl_debug_print() - GL ERROR: Source: Shader Compiler	Type: Error	ID: 0	Severity: High	Message: ERROR: 0:8: Syntax error, 'LIGHT_USE_RIM' macro redefinition

1: #version 100
2: #define ANDROID_ENABLED
3: #define SPECULAR_BLINN
4: #define ENABLE_COLOR_INTERP
5: #define ENABLE_TANGENT_INTERP
6: #define ENABLE_TANGENT_INTERP
7: #define LIGHT_USE_RIM
8: #define LIGHT_USE_RIM
9:
10: #ifdef USE_GLES_OVER_GL
11: #define lowp
12: #define mediump
13: #define highp
14: #else
15: precision highp float;
16: precision highp int;
17: #endif
18:

**ERROR**: SceneShaderGLES2: Vertex shader compilation failed:
Compile failed.
ERROR: 0:11: Syntax error, 'ENABLE_TANGENT_INTERP' macro redefinition
ERROR: 0:13: Syntax error, 'LIGHT_USE_RIM' macro redefinition
ERROR: 2 compilation errors. No code generated.

   At: drivers\gles2\shader_gles2.cpp:129:_display_error_with_code() - SceneShaderGLES2: Vertex shader compilation failed:
Compile failed.
ERROR: 0:11: Syntax error, 'ENABLE_TANGENT_INTERP' macro redefinition
ERROR: 0:13: Syntax error, 'LIGHT_USE_RIM' macro redefinition
ERROR: 2 compilation errors. No code generated.

**ERROR**: Method failed. Returning: __null
   At: drivers\gles2\shader_gles2.cpp:294:get_current_version() - Method failed. Returning: __null
**ERROR**: Condition "!version" is true. Returned: false
   At: drivers\gles2\shader_gles2.cpp:88:bind() - Condition "!version" is true. Returned: false
**ERROR**: Condition "!version" is true. Returned: -1
   At: drivers\gles2/shader_gles2.h:267:_get_uniform() - Condition "!version" is true. Returned: -1
**ERROR**: Condition "!version" is true. Returned: -1
   At: drivers\gles2/shader_gles2.h:267:_get_uniform() - Condition "!version" is true. Returned: -1
**ERROR**: Condition "!version" is true. Returned: -1
   At: drivers\gles2/shader_gles2.h:267:_get_uniform() - Condition "!version" is true. Returned: -1
**ERROR**: Condition "!version" is true. Returned: -1
   At: drivers\gles2/shader_gles2.h:267:_get_uniform() - Condition "!version" is true. Returned: -1
**ERROR**: Condition "!version" is true. Returned: -1
   At: drivers\gles2/shader_gles2.h:267:_get_uniform() - Condition "!version" is true. Returned: -1
**ERROR**: Condition "!version" is true. Returned: -1
   At: drivers\gles2/shader_gles2.h:267:_get_uniform() - Condition "!version" is true. Returned: -1

full log - full_log.zip
Steps to reproduce:
run attached project
Important note
I can reproduce this error only on PowerVR device.
I seems to not happen on other devices like Mali or VideoCore (or they are immune to it)
Minimal reproduction project:
shader in this project do really nothing, its just to trigger the error.
PowerVR-10.zip

Ah, this looks like it happens when we have multiple usage defines that map onto the same define. For example, if the user uses RIM, or RIM_TINT then LIGHT_USE_RIM. However, it appears that using both RIM and RIM_TINT defines LIGHT_USE_RIM twice. So the solution is going to be checking if the define has been done before doing it again here (and in all similar blocks):
https://github.com/godotengine/godot/blob/be5a47e75d4b671a48b0a35eb17887ee38d8516c/drivers/gles2/shader_compiler_gles2.cpp#L489-L498

Fixed by #38251.

tested, can confirm things work now as should. Thanks!


Revamp Bloom

Huge credit to @StarLederer, who did almost all of the work on this. We're just reusing this PR to keep everything in one place.
Objective

Make bloom energy conserving (optionally).
Make bloom work without HDR.
Improve artistic control.
Allow to use bloom as screen blur.
Fix #6634.
Fix viewport issues as described in #6802.
Address #2264.
Address #6655 (although the author makes incorrect conclusions).

Solution

Set the default threshold to 0, and lerp between bloom textures when composite_mode: BloomCompositeMode::EnergyConserving.
Use pipeline specialization to allow SDR textures as input when needed.
Use an innovative parametric function 🤓 to control blend levels for each bloom texture (can be customized per pixel using a texture in the future to achieve lens dirt and UI blur effects), and implement BloomCompositeMode::Additive` for situations where outdated look is desired.
Same as above.
Use render target alpha and discard bloom texture alpha.
Merge code from #6802.
Change default tonemap to an approximation of ACES RRT + sRGB ODT by default (not yet configurable, nor completely faithful to ACES guidelines, future PR).
See point no. 1.

Changelog

Bloom now looks different.
Bloom no longer requires an HDR camera, although it is heavily recommended.
Added BloomSettings:lf_boost, BloomSettings:lf_boost_curvature, BloomSettings::high_pass_frequency and BloomSettings::composite_mode.
BloomSettings::scale removed.
BloomSettings::knee renamed to BloomPrefilterSettings::softness.
BloomSettings::threshold renamed to BloomPrefilterSettings::threshold.
An approximation of ACES RRT + sRGB ODT is now the default tonemapping function.
The bloom example has been renamed to 3d_bloom and improved. A basic 2d_bloom example now exists.

Migration Guide

Refactor mentions of BloomSettings::knee and BloomSettings::threshold as BloomSettings::prefilter_settings where knee is now softness.
If defined without ..default() add ..default() to definitions of BloomSettings instances or manually define missing fields.
Adapt to Bloom looking visually different (if needed).
Adapt to small visual differences (if needed) due to the new default tonemapping function. Very bright colors will now desaturate towards white.

@JMS55 this fixes #2264. Could you add that to the PR description?

@james7312 mentioned that issue. What should I add to the migration guide?

A general guide on changes that need to be made. This one has an artistic impact so it may be harder to write, but the migration guide is intended as a way to document what is necessary to deal with the changes. This section is used to automatically generate the migration guide for the next version, so simply saying "adapt to X" is less of a guide and more of a command in this case. Either rewording it or providing concrete code examples on how to address the breaking changes would be for the best.

Thanks for the quick fix, imo ACES is a decent default 👍

A general guide on changes that need to be made. This one has an artistic impact so it may be harder to write, but the migration guide is intended as a way to document what is necessary to deal with the changes.

One thing that comes to mind is that ACES has a higher contrast, darker shades that looked fine before might get a bit too dark now. So making some lighting adjustments like lifting the ambient light brightness could help.
But it very much depends on the specific case I guess.
Also, small nit about the example:

the parameters didn't seem to be clamped, not sure if this is on purpose
the background color is rather bright, I think a darker one might work better

In a more complex scene I'm getting a bunch of black squares, that seem to mostly appear on top of dark objects. Possibly a floating point error (e.g. divide by zero) propagating through the downsampling/upsampling chain.

One thing I noticed is that it seems to be dependent on the aspect ratio of the window

Yeah I noticed that in my 4k monitor - it looks subtly different in 4k fullscreen vs 1080p windowed. I didn't think it was a huge deal at the time - most uses of bloom won't be on perfectly round spheres with a static camera. I didn't think about ultrawides though. I'll look into this, see if there's a way to fix it.

In a more complex scene I'm getting a bunch of black squares, that seem to mostly appear on top of dark objects. Possibly a floating point error (e.g. divide by zero) propagating through the downsampling/upsampling chain.

The article I was basing things on ran into the same issue (See "Bonus material 0" https://learnopengl.com/Guest-Articles/2022/Phys.-Based-Bloom). I didn't want to copy any "hacks" without a good test case to reproduce the issue, but now that it's come up I'll add it in my next set of commits. Please let me know if it fixes it, and thank you for testing it out! Also, how does the scene look otherwise?

In a more complex scene I'm getting a bunch of black squares, that seem to mostly appear on top of dark objects.

@pkupper can you verify if this has been fixed with my recent commits?

I tried experimenting with doing a fixed resolution and mipcount for the bloom texture. It just made the problem worse, unfortunately.

Think I might revert threshold_base -> threshold tomorrow, the more I think about it, the better that seems.

The clamping did indeed fix the artifacts. The ACES tonemapping did definitely improve the look of the bloom a lot, when comparing to the images from the release blog post. The defaults do sometimes look a bit strong (e.g. on the scooter in the second image).
Regarding the question of how the scene looks overall (not just bloom). IMO the most jarring issues in order of importance are:

Lack of mipmapping/anisotropic filtering (textures are really noisy, this is less visible in the screenshots)
Low resolution shadows (cascaded shadow maps are desperately needed)
Lack of global illumination (or at least ambient occlusion) to make the shadowed areas look less flat.
Transparency ordering issues.

@StarLederer @JMS55 we probably want to pull in the energy conserving aspect and so on from #6703 ?

I just closed my PR and will contribute here instead

we probably want to pull in the energy conserving aspect and so on from https://github.com/bevyengine/bevy/pull/6703 ?

This PR already makes one step towards that by interpolating the bloom image with the input image. To make it fully energy conserving all mips should be interpolated too (it looks like at the moment they are being added together). This PR removes the alpha channel, which I believe can be used to interpolate instead. That way we can also elliminate the need for texture_b.
view texture downsample into texture_a 0
texture_a 0 downsample into texture_a 1
...
texture_a n upsample and alphabblend into texture_ a n-1
...
texture_a 0 upsample and alphabblend into view texture

we probably want to pull in the energy conserving aspect and so on from https://github.com/bevyengine/bevy/pull/6703 ?

Also as a response to that, but separately from my previous comment, I'd like to propose a new, physically based technique to artistically control bloom that replaces threshold. Since bloom is the effect of light scattering in the lens and our goal is to be able to "tighten" the effect I think we can achieve that by tinting the lens and increasing the exposure. That way the light that travels through the lens more will loose energy while the photons that go straight through are brought back to their initial luminance by the increased exposure.
Of course, implementing that directly will result in large number multiplications and loss of color precision so instead we can simply tint the color at downsampling stage which will result in the same output

Since bloom is the effect of light scattering in the lens.

Your explanation of bloom somewhat contradicts the explanation from the video I posted in the other thread which claims that Fraunhofer diffraction (the light interfering with itself behind the aperture) is the cause. According to that video imperfections in the lens, while they do affect the result, are not actually needed for bloom to occur: https://youtu.be/QWqb5Gewbx8
Do you maybe have a source you can share for comparison? I would be interested in that.

Do you maybe have a source you can share for comparison?

My understanding of bloom (and aggressive passion for HDR + bloom + ACES) comes from Cody Darr (aka. Sonic Ether, autor SEUS). He had collaborated with Unity at some point and has removed some of his Unity packages when effects were implemented natively which suggests he might have been one of the co-authors of the old post processing package. He explains the effect in his Natural Bloom & Dirty Lens package user manual

@StarLederer I would love more help with bloom. Please feel free to contribute to this PR; you can open a PR on my fork adding commits to this branch. Note that this PR already gets rid of texture_b - it uses a single texture, reading from one mip and writing to the next.
Also, do you mind explaining how the alpha channel could be used to interpolate between mips, and why? If it's for performance, keep in mind that switching from R11g11b10 to Rgba16 will increase our GPU memory usage/bandwidth, which may lead to worse performance. Benchmarks in this area would be useful.
Besides the above discussions, here's a list of other ways we could improve bloom. I don't currently have time to work on any of them, but if you're interested, feel free to tackle them!

Trying to fix the resolution-dependent artifacts from upsampling (I couldn't figure this out, myself)
Using a compute shader instead of a fullscreen fragment shader, for better performance
Lens grime
WebGL compatibility/fallback (create and bind separate textures instead of using mips, also cannot use a compute shader if we do that)

Note that this PR already gets rid of texture_b

Oh, great, I took a look at the code but did not notice that.

Also, do you mind explaining how the alpha channel could be used to interpolate between mips, and why

Draw from mip n-1 to n using this blend when upscaling:
 blend: Some(BlendState {
  color: BlendComponent {
    src_factor: BlendFactor::SrcAlpha,
    dst_factor: BlendFactor::OneMinusSrcAlpha,
    operation: BlendOperation::Add,
  },
  // ...
}),

The fragment shader simply outputs intensity as alpha.
I thought it might be a more intuitive way to code this and, as i mentioned earlier, we need to interpolate between mips to get an energy conserving result, not add them. This might also be better for performance than sampling mip n and mixing with upsampled n-1 in the shader but this is definitely pulled out of ~my ass~ nowhere. I saw something about BlendFactor::Constant in the source code, no idea what that really is but if we can set an arbitrary value than setting the intensity and 1-intensity would be an even better solution. I'll try it out, but please leave a comment if you know how this works.

Trying to fix the resolution-dependent artifacts from upsampling

Definitely going to get on that since this is critical to energy conservation as well. Not really sure what my approach will be atm.

Lens grime

I'd leave this out of this PR but to get this effect we just need to use a texture instead of constant bloom intensity.

Using a compute shader

WebGL compatibility/fallback

These two i'd like to pretend don't exist for now.

Draw from mip n-1 to n using this blend when upscaling

So nevermind that, I just figured out that despite target format being 111110 you can still output alpha from fragment shader and use it in blending, which is what I have done to make bloom energy conserving.
Progress pic:

By the way, have not looked at tonemapping at all yet, but that definitely does not look like ACES

By the way, have not looked at tonemapping at all yet, but that definitely does not look like ACES

I assume you've set the saturation of the emissive spheres to 1.0? In that case it will not fade to white, regardless of the brightness. The tonemapping in this PR is using an ACES approximation. You can try decreasing the saturation very slightly.

The tonemapping in this PR is using an ACES approximation

Yeah, that is what i suspected

Something else we should address: Mention how to have high emittance 2d sprites/materials. Currently the docs only mention StandardMaterial::emissive for the 3d use case, but says nothing about 2d.

Finally! An update is here!
I've finished all changes I planned to make to this PR was authorized to merge them. Will update the PR description and post new screenshots tomorrow but here is a quick summary of what's new:

energy conserving mode
additive mode
compatibility with SDR cameras
extensive customization (can use bloom as free screen blur)
#6802
yet even fewer textures used internally

@JMS55 I cant edit your PR description. If you agree with changes, please update it with the following content:
![image](https://user-images.githubusercontent.com/55361752/210576184-2e2517f2-6a07-45b0-a52b-2aafd3fd73c5.png)

# Objective

1. Make bloom energy conserving (optionally).
1. Make bloom work without HDR.
1. Improve artistic control.
1. Allow to use bloom as screen blur.
1. Fix #6634.
1. Fix viewport issues as described in #6802.
1. Address #2264.
1. Address #6655 (although the author makes incorrect conclusions).

## Solution

1. Set the default threshold to 0, and lerp between bloom textures when `composite_mode: BloomCompositeMode::EnergyConserving`.
1. Use pipeline specialization to allow SDR textures as input when needed.
1. Use an [innovative parametric function 🤓](https://starlederer.github.io/bloom/) to control blend levels for each bloom texture *(can be customized per pixel using a texture in the future to achieve lens dirt and UI blur effects)*, and implement BloomCompositeMode::Additive` for situations where outdated look is desired.
1. Same as above.
1. Use render target alpha and discard bloom texture alpha.
1. Merge code from #6802.
1. Change default tonemap to an approximation of ACES RRT + sRGB ODT by default (not yet configurable, nor completely faithful to ACES guidelines, future PR).
1. See point no. 1.

## Changelog

* Bloom now looks different.
* Bloom no longer requires and HDR camera.
* Added `BloomSettings:lf_boost`, `BloomSettings:lf_boost_curvature`, `BloomSettings::high_pass_frequency` and `BloomSettings::composite_mode`.
* `BloomSettings::scale` removed.
* `BloomSettings::knee` renamed to `BloomPrefilterSettings::softness`.
* `BloomSettings::threshold` renamed to `BloomPrefilterSettings::threshold`.
* An approximation of ACES RRT + sRGB ODT is now the default tonemapping function.
* The bloom example has been improved.

## Migration Guide

* Refactor mentions of `BloomSettings::knee` and `BloomSettings::threshold` as `BloomSettings::prefilter_settings` where knee is `softness`.
* If defined without `..default()` add `..default()` to definitions of `BloomSettings` instances or manually define missing fields.
* Adapt to Bloom looking visually different (if needed).
* Adapt to small visual differences (if needed) due to the new default tonemapping function.

@StarLederer Fully reviewed the PR. Besides some random nits, looks good to me :)
Looking at the example though, the bloom effect when HDR is off doesn't seem right.

Added a barebones 2d bloom example.

Looking at the example though, the bloom effect when HDR is off doesn't seem right.

These spheres have emissive materials with 5-500 units of radiance per channel. They get clipped in SDR rendering so they are not going to look as glowy. Clipping happens before the image gets to the post-processing effects so all our effect "sees" is dim white spheres, our effect doesn't know they are supposed to be very bright and colorful.
I am actually surprised some color does get through... perhaps the material shaders apply the ACES-RRT-like function you implemented themselves when rendering to an SDR target? If that is the case that makes no sense physically and makes the bloom effect weirder than it could be... Regardless, there is no way around clipping, if it occurs, your scene is not going to look natural. There is a technique that I tried a while ago that sacrificed color precision for more dynamic range and it worked surprisingly well but that's a story for another day.
I don't think there is anything off about the bloom effect when in SDR it's just limitations of SDR.

Oh yeah, when not using HDR, tonemapping is applied in the main PBR shader, before bloom. That's a problem. Not sure how we fix that. Do we need to support bloom with SDR textures? I guess we could always use a separate tonemap node when BloomSettings is a component on the view, hmm. I'll think about it tomorrow.
Also, I just realized, we're going to have a hard time supporting WebGL. We can use separate textures in place of mips, but we can't use hdr, which is a bigger problem.

Is it WebGL 1 we are talking about?

WebGL2. Apparently there are issues with linear textures and MSAA on WebGL2, which is why it's not on by default :/
What do you think of removing the SDR stuff for now, merging this PR, and then trying to add that later? I don't want to block all the other improvements we've made because of SDR support.

have a linear_output toggle on the PBR system instead of relying on camera.hdr to determine user's intentions.

Yes. Imo HDR is a misleading name for the feature - it's not really about the user's display. Ideally we'd only have HDR as well imo, it would simplify a good amount of things.

What do you think of removing the SDR stuff for now

I don't mind but is it really a blocker? At worst we are going to get the look that half ReShade presets create and many people love it.

Ideally we'd only have HDR as well imo

Yes, I'm not suggesting we remove that one, they are both useful options that correspond to different things.

it's not really about the user's display

Not sure I agree with this part since by HDR cameras most engines imply that a camera's render target (not user display) is HDR. The user display should probably be configured in window options and in (faux) post-processing (tonemapping should be different for different display types)

I don't mind but is it really a blocker?

Doing tonemapping before bloom in SDR seems wrong, and fixing it would be slightly complicated at the moment. Easier to just disable it for now as most people will want to use HDR anyways.

The user display should probably be configured in window options and in (faux) post-processing (tonemapping should be different for different display types)

I was researching tonemapping a few weeks ago. The conclusion I came to was that's it's complicated, I didn't understand it well, and wouldn't provide a ton of benefit atm (not many people have an HDR or P3 monitor). Feel free to take charge on it :)
In other news, something seems to have broken bloom when I rebased :(

Easier to just disable it for now

Fair enough, that will also prevent some confusion. We need to add the camera.hdr check to one of the extraction systems then and possibly add '(not yet lol)' to all comments that mention ability to use bloom in SDR.

The conclusion I came to was that's it's complicated

I might be wrong about some of this but I have done research on the topic before too. Here is my understanding:
In an ideal ACES workflow you perform your shading calculations in ACES AP0 color space (or more commonly in linearazed sRGB colorspace and map to AP0 just before tone-mapping) and, when it's time to output, you apply the Reference Rendering Transform (RRT), followed by an Output Device Transform (ODT) that corresponds to the output device and in some cases even the surroundings (how the room is lit). Of course, most of the time it is not really worth it and you just hardcore everything together into a function that combines conversion from (hopefully) linearized sRGB to ACES AP0, RRT and sRGB ODT. Unity does this by default, unfortunately their (very good) implementation is under this sketchy license so I don't know if we can just copy it. Unity Core Rendering Pipeline also includes a spec-compliant implementation which is under a free license (see file) but it is probably slower. This paragraph might belong in a new PR btw.

Feel free to take charge on it :)

I'm a little busy right now because I need to find a job (and update my portfolio) so have no idea how much time I'll have in the upcoming months but I'd absolutely love to.

Doing tonemapping before bloom in SDR seems wrong, and fixing it would be slightly complicated at the moment. Easier to just disable it for now as most people will want to use HDR anyways.

Wouldn't keeping bloom in SDR allow it to somewhat works in Wasm?

I might be wrong about some of this but I have done research on the topic before too. Here is my understanding:
In an ideal ACES workflow you perform your shading calculations in ACES AP0 color space (or more commonly in linearazed sRGB colorspace and map to AP0 just before tone-mapping) and, when it's time to output, you apply the Reference Rendering Transform (RRT), followed by an Output Device Transform (ODT) that corresponds to the output device and in some cases even the surroundings (how the room is lit). Of course, most of the time it is not really worth it and you just hardcore everything together into a function that combines conversion from (hopefully) linearized sRGB to ACES AP0, RRT and sRGB ODT. Unity does pretty much that, unfortunately their (very good) implementation is under this sketchy license so I don't know if we can just copy it.

How about using Baking Lab's ACES tonemapper? It's very straight forward and produces nice results in my opinion. It's also under the MIT license, so that shouldn't be a problem.
I'm not sure if this implements the exact color transformation you've mentioned, but I think over-complicating things for the sake of accuracy won't be of much use for most bevy users.

How about using Baking Lab's ACES tonemapper?

I don't think this is much better than what @JMS55 already implemented. It looks like it is just the S curve without the glow module, red transform and global desaturation. That function will never look AAA, creates visual artifacts with very bright and very saturated colors, and blocks us from supporting HDR (or any non sRGB) display in the future.

I think over-complicating things for the sake of accuracy won't be of much use for most bevy users

It's not for the sake of accuracy, ACES RRT is is what the greatest color grading experts concluded looks best and I think it's smart to use their expertise The Academy made available for free. A proper ACES tonemap is not that complex if you look at their reference implementation (which i probably should have mentioned instead Unity), all you need is to be in ACES AP0 which is achieved by a simple matrix multiplication, then execute this (RRT), then this (ODT) if you target normal monirots or this (ODT) if you target HDR ones.
We can technically improve performance by converting everything in RRT to work on lineraized sRGB but that is a lot of work and technical debt. Alternatively, we can bring the entire color workflow into ACES (textures need to be converted to ACES AP0 instead of linear sRGB) upon load. Or at least I would just follow the steps I mention above.

Wouldn't keeping bloom in SDR allow it to somewhat work in Wasm

Bloom still uses HDR textures internally no matter the camera target

How about using Baking Lab's ACES tonemapper?

I don't think this is much better than what @JMS55 already implemented. It looks like it is just the S curve without the glow module, red transform and global desaturation. That function will never look AAA, creates visual artifacts with very bright and very saturated colors, and blocks us from supporting HDR (or any non sRGB) display in the future.

I'm not quite sure what artifacts you are taking about. If you're referring to saturated highlights not fading to white, that is actually not the case with the code I've linked since it doesn't apply the tone curve in linear Srgb.
Also, are there any resources on what glow module, red transform and global desaturation are supposed to mean? I've never heard of those and a quick search resulted in little to no information.
Though at this point I don't want to bloat the comments on this PR even more, as the implementation isn't up to me anyways.

Though at this point I don't want to bloat the comments on this PR even more

I agree that we are getting way off topic. Let's move the tonemapping conversation over to #7195

Also, are there any resources on what glow module, red transform and global desaturation are supposed to mean

Not really, all you have to go by is the reference implementation

According to that video imperfections in the lens, while they do affect the result, are not actually needed for bloom to occur: https://youtu.be/QWqb5Gewbx8

Just found a video on how that (convolution bloom) can be implemented in real time. This is still very heavy and not feasible for now but might become common within next 5 years.

This has issues when trying to run on DX12 due to BloomUniforms not being mapped.
INFO naga::back::hlsl::writer: Skipping global [3] (name Some("uniforms")) for being inaccessible: mapping of ResourceBinding { group: 0, binding: 2 } is missing
INFO naga::back::hlsl::writer: Skipping function [9] (name Some("soft_threshold")) because global [3] is inaccessible

bloom.wgsl:
@group(0) @binding(2)
var<uniform> uniforms: BloomUniforms;

@StarLederer Sometime soon, I plan to remove the SDR stuff and rebase the code so that we can merge this in time for bevy 0.10. I'll fork it into a separate branch first, in case you want to add back SDR support in the future.

This has issues when running on DX12 due to BloomUniforms not being mapped.

What does this mean? "Not being mapped"? Does it mean not bound? Though the errors are from naga...

This is quite a big PR with multiple things being changed at once. That makes review more difficult as the context is "do things to make bloom better" rather than "address this one issue with bloom" and a series of PRs like that that are small, clear, and easy to review. Each of the points in the solutions section in the description look to be logically-separate concerns.
And to be clear, I know it is difficult to know what ideas are going to produce good results and not when working on rendering code and you have to put a lot of things into place to find out what ideas you want to keep and what you want to remove. That leads to branches with multiple features implemented on top of each other and that perhaps interact with each other. However, when it comes to reviewing, it is important to keep things as small as one can. I myself have gone through the process of splitting out bits and pieces from branches I've hacked on for the sake of making reviewing easier. I'd strongly encourage this approach and more things will get merged more quickly by doing things this way.

in case you want to add back SDR support in the future

I thought that'd be a sure thing. I was procrastinating disabling HDR support myself mainly because it is the comments that need to be changed and not code.
I also agree with @superdump. One more advantage of doing things that way is that it makes it easier to rebase (e.g. I had to resolve tonemapping related conflicts in the PBR shader when working on bloom)

Fixed all the references. I'm unsure how the slide numbers got messed up, but I've corrected them against the reference link I put in the shader.
Also fixed the 0.25, thanks for noticing. No wonder bloom looked weird to me yesterday when I was testing something :P.
All that's remaining is double checking the LearnOpenGL article we're basing our code off of is correct. I've left those discussions up for now.

I've gone through the shader parts of the learnopengl article and compared. I also checked the sampler and texture configurations. I haven't checked the rest of the GL API code as I'm more confident about the data flow already.
The only thing I noticed that was 'missing' was the 'Bonus Material 0: Graphical Debugging' part about the black boxes. The solution to that problem was to add a max(sample, 0.0001) at the end of the first downsample pass to make sure no multiply by zeros were introduced that propagate into black boxes. I don't know if we've experienced that at all or if it doesn't happen for some other small detail in what we have implemented. Instead we seem to clamp between 0.0 and what looks to be possibly the largest representable float. I think I'll add in the 0.0001 as the lower bound of the clamp, for safety.

Removed the ACES change. Left a comment in the BloomSettings docs to suggest a good tonemapper.

Some of the bloom “shapes“ now seem to be duplicated/triplicated horizontally for me, is this due to the firefly prevention mechanism?

The examples are currently broken, I get an error about view not being in scope when enabling a different tonemapper.
color = saturation(color, view.color_grading.pre_saturation);

@robtfm came up with a great improvement to make bloom look more consistent at various resolutions.
The only thing really missing now is the glowing bevy logo in the bloom_2d example looks kinda bad. Rather than adjust the color values in code, it would be better to encode the image such that the white birds are bright, but the black circle they are on aren't. The way it is now, the whole thing kinda fades to white and looks bad and washed out. Not a huge blocker though, and we could just remove it. We've had questions in the past on how to add bloom to a 2d sprite, so that was my attempt at trying to show it. Didn't work out that well.


equals method in SegmentId looks buggy

Coordinator fails to distribute segments and suspect the equals method in SegmentId caused this issue.
  private int computeHashCode()
  {
    // Start with partitionNum and version hash codes, because they are often little sequential numbers. If they are
    // added in the end of the chain, resulting hashCode of SegmentId could have worse distribution.
    int hashCode = partitionNum;
    // 1000003 is a constant used in Google AutoValue, provides a little better distribution than 31
    hashCode = hashCode * 1000003 + version.hashCode();

    hashCode = hashCode * 1000003 + dataSource.hashCode();
    hashCode = hashCode * 1000003 + Long.hashCode(intervalStartMillis);
    hashCode = hashCode * 1000003 + Long.hashCode(intervalEndMillis);
    hashCode = hashCode * 1000003 + Objects.hashCode(intervalChronology);
    return hashCode;
  }

  @Override
  public boolean equals(Object o)
  {
    if (this == o) {
      return true;
    }
    if (!(o instanceof SegmentId)) {
      return false;
    }
    SegmentId that = (SegmentId) o;
    // Compare hashCode instead of partitionNum: break the chain quicker if the objects are not equal. If the hashCodes
    // are equal as well as all other fields used to compute them, the partitionNums are also guaranteed to be equal.
    return hashCode == that.hashCode &&
           dataSource.equals(that.dataSource) &&
           intervalStartMillis == that.intervalStartMillis &&
           intervalEndMillis == that.intervalEndMillis &&
           Objects.equals(intervalChronology, that.intervalChronology) &&
           version.equals(that.version);
  }

Affected Version
0.16.0
Description
We got exceptionMessage=Can't add chunk[org.apache.druid.timeline.partition.LinearPartitionChunk@1cb644f] to a full atomicUpdateGroup[AtomicUpdateGroup{chunks=[org.apache.druid.timeline.partition.LinearPartitionChunk@9a644f]}]} error from Coordinator when we were trying upgrading our cluster from 0.13.0 to 0.16.0, refer Stack trace for details.
To give more contexts here, we are using our own indexer which reads data from Kafka and builds segments. We use LinearShardSpec for creating our segments and maintain an internal logic to generate the partition number base on our data distribution strategy, the generated partition number per interval is not that consecutive, e.g. following partition numbers are generated for the same interval, and we may produce 2000 to 4000 segments in our production.
10112217, 100412240, 110112589, 120212564, 20110920, 30212707, 40112686, 50312650, 60212319, 70412079, 80212020, 90312664

Cluster size
We found this on our dev cluster which has only 3 historical nodes, 1 query broker and 1 coordinator.

Steps to reproduce the problem
Not able to reproduce the problem since we found this issue when we were trying to upgrade our DEV cluster to 0.16.0 and the segments may have been cleaned up.

Any debugging that you have already done

Went through VersionedIntervalTimeline, OvershadowableManager, AtomicUpdateGroup and RootPartitionRange classes, for the same interval, version, we should not have multiple segments which have LinearShardSpec belong to the same AtomicUpdateGroup, since the RootPartitionRange for LinearShardSpec is always (partitionNum, partitionNum+1), we should never have same partition number for the same interval (guaranteed by DB table constraint).
In doPoll() method of SQLMetadataSegmentManager, every time when we query DB, we try to replace a segment by an existing one in previous snapshot if we can find it by SegmentId. If there is any collision can be occurred for 2 different partition numbers (the rest factors are the same) from computeHashCode() method of SegmentId class, our segment poll is broken.
Unfortunately we couldn't reproduce the issue, but I still don't feel comfortable to use a hash for a equals method of an Java Object, especially when it's being used as a key-lookup, in my poor opinion, comparing partitionNum there would be better (considering performance vs correctness, and I am not sure how much performance we could improve by comparing the hash).

Stack trace

2019-11-13T01:43:23,526 ERROR [org.apache.druid.metadata.SQLMetadataSegmentManager-Exec--0] org.apache.druid.metadata.SQLMetadataSegmentManager - Uncaught exception in class org.apache.druid.metadata.SQLMetadataSegmentManager's polling thread: {class=org.apache.druid.metadata.SQLMetadataSegmentManager, exceptionType=class org.apache.druid.java.util.common.ISE, exceptionMessage=Can't add chunk[org.apache.druid.timeline.partition.LinearPartitionChunk@1cb644f] to a full atomicUpdateGroup[AtomicUpdateGroup{chunks=[org.apache.druid.timeline.partition.LinearPartitionChunk@9a644f]}]}
org.apache.druid.java.util.common.ISE: Can't add chunk[org.apache.druid.timeline.partition.LinearPartitionChunk@1cb644f] to a full atomicUpdateGroup[AtomicUpdateGroup{chunks=[org.apache.druid.timeline.partition.LinearPartitionChunk@9a644f]}]
    at org.apache.druid.timeline.partition.OvershadowableManager.addChunk(OvershadowableManager.java:656) ~[druid-core-0.16.0-incubating-iap3.jar:0.16.0-incubating-iap3]
    at org.apache.druid.timeline.partition.PartitionHolder.add(PartitionHolder.java:61) ~[druid-core-0.16.0-incubating-iap3.jar:0.16.0-incubating-iap3]
    at org.apache.druid.timeline.VersionedIntervalTimeline.addAll(VersionedIntervalTimeline.java:188) ~[druid-core-0.16.0-incubating-iap3.jar:0.16.0-incubating-iap3]
    at org.apache.druid.timeline.VersionedIntervalTimeline.addSegments(VersionedIntervalTimeline.java:116) ~[druid-core-0.16.0-incubating-iap3.jar:0.16.0-incubating-iap3]
    at org.apache.druid.timeline.VersionedIntervalTimeline.forSegments(VersionedIntervalTimeline.java:107) ~[druid-core-0.16.0-incubating-iap3.jar:0.16.0-incubating-iap3]
    at org.apache.druid.timeline.VersionedIntervalTimeline.forSegments(VersionedIntervalTimeline.java:100) ~[druid-core-0.16.0-incubating-iap3.jar:0.16.0-incubating-iap3]
    at org.apache.druid.client.DataSourcesSnapshot.lambda$new$3(DataSourcesSnapshot.java:84) ~[druid-server-0.16.0-incubating-iap3.jar:0.16.0-incubating-iap3]
    at org.apache.druid.utils.CollectionUtils.lambda$mapValues$0(CollectionUtils.java:96) ~[druid-core-0.16.0-incubating-iap3.jar:0.16.0-incubating-iap3]
    at java.util.HashMap.forEach(HashMap.java:1289) ~[?:1.8.0_181]
    at org.apache.druid.utils.CollectionUtils.mapValues(CollectionUtils.java:96) ~[druid-core-0.16.0-incubating-iap3.jar:0.16.0-incubating-iap3]
    at org.apache.druid.client.DataSourcesSnapshot.<init>(DataSourcesSnapshot.java:82) ~[druid-server-0.16.0-incubating-iap3.jar:0.16.0-incubating-iap3]
    at org.apache.druid.client.DataSourcesSnapshot.fromUsedSegments(DataSourcesSnapshot.java:54) ~[druid-server-0.16.0-incubating-iap3.jar:0.16.0-incubating-iap3]
    at org.apache.druid.metadata.SQLMetadataSegmentManager.doPoll(SQLMetadataSegmentManager.java:973) ~[druid-server-0.16.0-incubating-iap3.jar:0.16.0-incubating-iap3]
    at org.apache.druid.metadata.SQLMetadataSegmentManager.poll(SQLMetadataSegmentManager.java:905) ~[druid-server-0.16.0-incubating-iap3.jar:0.16.0-incubating-iap3]
    at org.apache.druid.metadata.SQLMetadataSegmentManager.lambda$createPollTaskForStartOrder$0(SQLMetadataSegmentManager.java:321) ~[druid-server-0.16.0-incubating-iap3.jar:0.16.0-incubating-iap3]
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) [?:1.8.0_181]
    at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308) [?:1.8.0_181]
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180) [?:1.8.0_181]
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294) [?:1.8.0_181]
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [?:1.8.0_181]
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [?:1.8.0_181]
    at java.lang.Thread.run(Thread.java:748) [?:1.8.0_181]

cc: @michaelschiff

Unfortunately we couldn't reproduce the issue, but I still don't feel comfortable to use a hash for a equals method of an Java Object, especially when it's being used as a key-lookup, in my poor opinion, comparing partitionNum there would be better (considering performance vs correctness, and I am not sure how much performance we could improve by comparing the hash)

If you think this issue is down specifically to SegmentId implementation, could you try to construct a unit test (with your LinearShardSpec) which demonstrates the inconsistency of equals() and hashCode(), or that hashCode() is buggy?

I'm also not certain that the equals method is correct as written
The hashcode is calculated via: (comments removed for readability)
int hashCode = partitionNum;
hashCode = hashCode * 1000003 + version.hashCode();
hashCode = hashCode * 1000003 + dataSource.hashCode();
hashCode = hashCode * 1000003 + Long.hashCode(intervalStartMillis);
hashCode = hashCode * 1000003 + Long.hashCode(intervalEndMillis);
hashCode = hashCode * 1000003 + Objects.hashCode(intervalChronology);

and the comment from equals:
// Compare hashCode instead of partitionNum: break the chain quicker if the objects are not equal. If the hashCodes
// are equal as well as all other fields used to compute them, the partitionNums are also guaranteed to be equal.

That logic If the hashCodes are equal as well as all other fields used to compute them, the partitionNums are also guaranteed to be equal. will hold over an infinite field, but over a finite field (like int) it is possible that you can start with two different partition numbers, multiply them by the same sequence of numbers, and arrive at the same answer % 2^32.
Take as a simple example, partitions x & y, where x != y and there is only one other "field" used to compute the hash-code (which in this example is held constant) and we are using the finite field Mod 8
x = 1
y = 3

x * 4 Mod 8 = 4

y * 4 Mod 8 = 4

While a full size counter example is constructed, I want to point out that this discussion could be obviated by adding an additional comparison to the equals method (checking the partition number too).  The comparison of the hashcode can be left in for quicker exit still (since if the hashcodes are different, they are certainly different), but in the case of a hashcode collision all fields needs to be checked.

Im no longer convinced that the logic is incorrect.
a ≡ b (mod m) iff m | (a - b)

Taking a simple example with only a single other field, whose hashcode is k, we have
((a * 1000003) + k) ≡ ((b * 1000003) + k) (mod 2^32)

using the above becomes
2^32 | ((a * 1000003) + k) - ((b * 1000003) + k)

2^32 | (a * 1000003) - (b * 1000003)

2^32 | 1000003 * (a - b)

since 1000003 and 2^32 are coprime, their least common multiple is their product.  Given that, the only way the above can hold is if (a-b) is a multiple of 2^32. This is not possible if a and b are both int.
All else equal, I still think it makes sense to include the partition number comparison.  Thinking through this level of modular arithmetic to prove an equals() method correct feels too clever to me.

@michaelschiff thanks for looking into this. I did a simple math and also confirmed this should not be an issue.
@leventov
I am able to reproduce the issue now, we got the Exception and could not start our Coordinator, the root cause is - When constructing RootPartitionRange, our partition numbers are cast from Integer to short, hence the collision occurred (our partition numbers are not that consecutive and may have large gap between each other). Checked NumberedPartitionChunk, NumberedOverwritingPartitionChunk, LinearPartitionChunk and IntegerPartitionChunk, the partition numbers are defined as int, should we consider to make the startPartitionId and endPartitionId to int and update corresponding codes.
    private final short startPartitionId;
    private final short endPartitionId;

    @VisibleForTesting
    static RootPartitionRange of(int startPartitionId, int endPartitionId)
    {
      return new RootPartitionRange((short) startPartitionId, (short) endPartitionId);
    }

    private static <T extends Overshadowable<T>> RootPartitionRange of(PartitionChunk<T> chunk)
    {
      return of(chunk.getObject().getStartRootPartitionId(), chunk.getObject().getEndRootPartitionId());
    }

Hi @ArvinZheng, thank you for the report. In https://github.com/apache/incubator-druid/issues/7491, a new assumption has been added that partitionId is never greater than Short.MAX_VALUE to reduce the memory usage in the coordinator and the broker. This assumption makes sense to me since partitionId is assumed to be consecutive in most cases and having such a large number of segments in a time chunk generally doesn't happen in practice. I guess this assumption doesn't hold for your use case. What kind of ingestion system are you using? And why are partitionIds very sparse?

hi @jihoonson , thank you for looking into this. Yah, I just realized this limitation was introduced by the minor compaction. No, the assumption doesn't hold for our use case, as I described, we created our own Indexer based on core Druid libraries.
Our Indexer pulls events from Kafka and split them into small slices to further improve the parallelism, i.e.

Our service coordinator pulls 6M events from Kafka per partition per Batch, and split this batch into 6 small Slices.
Then we send this 6 slices to downstream service which is our indexer, and each indexer creates segments independently.
To avoid having collision and creating locks between our indexer instances, we created a partition number generator which creates the partition number base on Kafka Partition Number, Slice Number and Kafka Batch Number (maintained by our coordinator), sample rule as follows,

Kafka Partition Number * PARTITION_NUMBER_MULTIPLIER + Slice Number * SLICE_NUMBER_MULTIPLIER + Kafka Batch Number % SLICE_NUMBER_MULTIPLIER

Hence we have large numeric values for our partition numbers even we may not have so many segments per interval.
I understand it's not trivial to modify current minor compaction, but we should state this situation clearly on the release doc - https://github.com/apache/incubator-druid/releases/tag/druid-0.16.0-incubating, so that people can do some pre-checks on their system. Happy to discuss anything. :)

Thanks for the suggestion. It sounds a good idea. I will update release doc sometime soon.


Fail on unknown JDBC type

Fail on unknown JDBC type

I propose a different new behavior:

expose all columns, including those unsupported (perhaps as unknown type)
fail query only when it accesses unsupported columns (either explicitly, or SELECT *)

Regarding default behavior, there are different use-cases for SELECT *:

copying/migrating data (usually CREAT TABLE .. AS SELECT * FROM ..)

in this case, the new behavior is more appropriate

poking around (usually SELECT * FROM .. LIMIT 1)

in this case, the current behavior is more appropriate

Since current behavior is more appropriate in certain use-cases, I would not change current default behavior.

What about conversion unknown types to varchar? I think we could add only a session property that would be always set to false (no config property to overwrite that). So user could peek the data, knowing the fact that column data type is unsupported and it may change in future.
@findepi What do you think?

Seems reasonable to me.

Replaced with #1182


[allow-proper-branch-names] 🛢 Use custom BranchField for branches

 Bug: Project/Task branch_name fields use SlugField with invalid characters.

 I'll see where that's coming from shortly.

@davisagli we could, but I felt that a name that communicated what kind of field this was (à la how SlugField does so) would be more felicitous. I'm not attached to it, but I do think it's a nicer way of doing it.

@wlonk I think it makes it harder to read the model code and understand it. I know from experience how a CharField works; I don't know how a BranchField works without digging around to find the code. That's exacerbated by moving the defaults for max_length, null, and blank to the field class instead of the line where the field is instantiated.
In addition to the readability concerns, I also have concerns that a custom field class will require more work to maintain over time. I think that extra work is warranted for cases where we're actually storing a different type (JSONField, TextField) but I'm not seeing any big payoff for it here.

@davisagli that's enough to convince me. I will change it.


feat(helm): copy helm charts from external repository

Copies helm charts from aiven-charts.

The other repository was also set up as a helm repository via github actions, do we do this here too?

The other repository was also set up as a helm repository via github actions, do we do this here too?

Yes. The idea to automate charts releasing process (patching). I have already made a "script" for that. But I do want to have clean diff rather than putting altogether in one huge messy PR.
So this, and this are preparations for a real feature.

Ok great! I intially did set it up as external repo since i was thinking that we might have other helm charts too ( for karapace i.e. ) but if we only have the operator it makes more sense to have it grouped here!

Ok great! I intially did set it up as external repo since i was thinking that we might have other helm charts too ( for karapace i.e. ) but if we only have the operator it makes more sense to have it grouped here!

Yeah, that would make sense.
Just want to reduce risks on copy-pasting things. And eventually I want to release thing altogether automatically using version number, rather than putting that manually everywhere.


This card action is disabled because you've blocked .

The Chat with Button and any other deeplinks within the Icebreaker adaptive cards are not working for Guest User. When the card is initially sent to the Guest User Account, first time the user clicks it works, but subsequest times, the following error message shows up below the card and nothing happens.
"This card action is disabled because you've blocked ."

@dharmesh-pariawala Thank you for your query. We are looking into it.

HI @dharmesh-pariawala ,
Thank you for submitting this issue, as we work to ensure consistent responses with items logged in our issues list, we are working to address these items that did not get responses. Please let us know if you are still facing this issue and require assistance and we will be happy to discuss. If no further action is needed at moment and no response is received, we will close out the issue in 3 business days. You may also always open a new issue or respond here if something does come back up.

I can report the exact same bug for a user using a standard account. He was able to do a first click but got the same error message for the second. Tested on Electron app of MS Teams.

Same issue here. Several users have been consistently reporting running into this issue when trying to click "Propose Meetup" or "Chat with"

Hi we're getting this issue as well - any updates on a fix or workaround? thanks

Same issue here. Any news on this case?

What's the fix for this? I've just implemented it and we have 8 testers who are on the same policies but 2 members are getting this message.

We also have a number of user's that have this issue. Is there a fix?

I've asked those having issue to add the app first then the error disappeared. @teiselt96, can you try that and confirm if that's the possible solution?

What do you mean by "add the app first" we already have it deployed to a Microsoft Team.

What do you mean by "add the app first" we already have it deployed to a Microsoft Team.

Unsure on whether you pushed it to your users thru policy but if not, ask one of those affected user to add the app in their Teams desktop app first

Getting the same issue for some users. Is there any confirmed fix for this?

Hi, Team,
Just wanted to mention this is still occurring. I've got a user who was able to use Icebreaker on his old computer. Got a new computer and received this message--he can still access Icebreaker on his phone, but not from his computer.

Hi, can we have an update on this please? @v-royavinash

@kf-paulbrown Could you please log an new issue with complete details on the unexpected behavior It will be helpful for us to track it in a better way.

I have a customer who is still facing this issue.

we are noticing same issue to few users. Please share if any fix is identified

I also have a user seeing this. (We are not using the latest version of Icebreaker.)


[Bug]: Social links not working in the footer of the website.

What happened?
A bug happened! The social links of Twitter, YouTube, Github, and Slack aren't working in the footer of the website.
They are working in the mobile view but not in the desktop view.
To Reproduce

Go to... https://www.featbit.co/
And scroll down to its footer.

Expected behavior
The icons/buttons should redirect to their proper links when clicked on the website on the desktop.
Additional context

Code of Conduct

[X] I agree to follow this project's Code of Conduct

Please assign me this issue. I would like to work on it and fix this.

cc @cosmic-flood

Hi @niranjan-kurhade
Thank you for your feedback. I left you a message on Slack. Actually, the homepage project is independent and in a private repository.
It's kind of hard to make the project public today.  I don't think I can allow you to contribute to it. But thanks for the question. Much love to your contribution.

Fixed in private repo for home page. Thank you again @niranjan-kurhade


AVG-RECEIVE-BYTES-NUM-DELTA-BY-CLIENT-NUM doesn't add up?

AVG-RECEIVE-BYTES-NUM-DELTA-BY-CLIENT-NUM.csv numbers doesn't add up correct...
Investigate this.

@heyitsanthony I am adding interpolate feature in psn side with more test cases.
Then we will be ready to run new set of tests with new data.
I will investigate on this once we have that new data from the new code.


Styling

This update adds the styling meeting the following requirements:

The final design should match Zeplin's design (this includes all elements + the same styling)
Used the CSS rules detailed in Zeplin to style the website.

Invalid Code Review Request ❌
Hi @zamcham,
While you made a great effort in this project, unfortunately, I cannot proceed to review your code.
You have submitted a project, that doesn't have the correct initial setup. You have used vite but it is requested to use react-scripts. Sorry for this conflict and please use [this link](https://create-react-app.dev) to set up your project:
Your Code Review Request will be marked as invalid in your Dashboard, so please submit a new one once you are ready 🙏
Cheers and Happy coding!👏👏👏
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me in your question so I can receive the notification.
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.

Invalid Code Review Request does not count into the code reviews limit.


Difference between this repo and TomCools/dropwizard-websocket-jee7-bundle?

In the docs you show an alternative to your project is:
https://github.com/TomCools/dropwizard-websocket-jee7-bundle
What's the difference?

The alternative project didn't implemented websocket instrumentation needed to support metrics, for example.


chore: fix some comments

Fixes Or Enhances
Make sure that you've checked the boxes below before you submit PR:

[x] Tests exist or have been written that cover this particular change.

@go-playground/validator-maintainers

coverage: 74.208%. remained the same
when pulling 50cb4f48c897ea9fea8fa7b8746b506b81c2c483 on pavedroad:master
into a0f74b0fb2a7ae1750c0f0b0a49550d8b6e2e708 on go-playground:master.

friendly ping~


Fixed handling of Link targets

Since Links now use a weakrefdict to store handles on the source and target objects it is possible for the reference to be deleted before the plot is instantiated. In cases like that the plots should not attempt to instantiate the link callback.

Makes sense. Happy to merge when the tests pass (I am assuming there are some unit tests for links).

(I am assuming there are some unit tests for links)

There are, I had considered adding new ones but didn't get the garbage collection to work consistently to actually test for the issue this is meant to prevent.

Tests are now green. Merging.


@conda-forge-admin please add bot automerge

@conda-forge-admin please add bot automerge

Hi! This is the friendly automated conda-forge-webservice.
I just wanted to let you know that I added bot automerge in conda-forge/tinyrpc-feedstock#13.


raftstore-v2: publish tablet in raftstore thread only

What is changed and how it works?
Issue Number: Ref #12842
What's Changed:
Publish tablet in apply thread is unsafe. This PR moves the operation to
raftstore. It also fixes the issues that applying two splits at a time can
cause panic. It also makes sure cache will be cleared after tablet is published.

Check List
Tests

Unit test
Integration test

Release note
None

PTAL

//merge

/merge


Update OpenAI_On_Python.md

I added my application of open ai in which i explain the basic start of usage of Open Ai in python using openAI libraries.

Hey i removed the pre requisites part


can i get like this view

http://i.imgur.com/UsRIDlO.png

Hi. This is my next library. Just wait. Thanks.

k
i wanted icarousel like view

Yeah yeah. Like thia.

d u knw any segmented control in android like ios

No. Currently nothing.

ok


feat: add TLS endpoint to kepler exporter

Add TLS support via the new web configuration file, following the Prometheus Exporter Toolkit style for consistency across exporters.

Usage:

kepler --web.config.file=web-config.yml

Content of web-config.yml:

tls_server_config:
cert_file: /path/to/server.crt
key_file: /path/to/server.key

Hi @SamYuan1990 ,
I wanted to clarify something regarding your comments about Prometheus official support. Could it be that there was a mix-up between the terms "rolled out" and "rules out"?
While I don't claim expertise in Kubernetes security or managing certificates in multi-node deployments, I can point out that some IaaS platforms, like Red Hat OpenStack (RHOSO), are already using node_exporter with TLS. Here are a couple of links to highlight this:
Node Exporter Configuration
Node Exporter Setup Tasks

Furthermore, they are also using Kepler, as shown here:
Kepler Integration Tasks
This leads me to my point: if someone knows how to manage their certificates effectively, Kepler should be capable of supporting TLS endpoints. Why should Kepler be the only exporter in an IaaS setup like OpenStack that does not support TLS, especially when node_exporter, ipmi_exporter, and others do?
Additionally, Kepler is handling sensitive data. For instance, consider the CVEs related to RAPL and side-channel attacks, which I’m sure you’re familiar with. Would organizations that prioritize security really allow this data to be transmitted in plaintext over the network?
Finally, I want to stress that this is an optional feature. For users content with unencrypted data, nothing changes—they can continue as before. But adding this option would present Kepler as a more robust and serious tool for those who care about security.

Hi @SamYuan1990 , I wanted to clarify something regarding your comments about Prometheus official support. Could it be that there was a mix-up between the terms "rolled out" and "rules out"?
While I don't claim expertise in Kubernetes security or managing certificates in multi-node deployments, I can point out that some IaaS platforms, like Red Hat OpenStack (RHOSO), are already using node_exporter with TLS. Here are a couple of links to highlight this:
Node Exporter Configuration Node Exporter Setup Tasks  Furthermore, they are also using Kepler, as shown here: Kepler Integration Tasks
This leads me to my point: if someone knows how to manage their certificates effectively, Kepler should be capable of supporting TLS endpoints. Why should Kepler be the only exporter in an IaaS setup like OpenStack that does not support TLS, especially when node_exporter, ipmi_exporter, and others do?
Additionally, Kepler is handling sensitive data. For instance, consider the CVEs related to RAPL and side-channel attacks, which I’m sure you’re familiar with. Would organizations that prioritize security really allow this data to be transmitted in plaintext over the network?
Finally, I want to stress that this is an optional feature. For users content with unencrypted data, nothing changes—they can continue as before. But adding this option would present Kepler as a more robust and serious tool for those who care about security.

From security point of view, yes. We can make kepler protected by TLS to improve our baseline for sensitive data protection.
Once more, this PR is a great job and I just "mark with change request for more information and discussion."
So from deployment point of view, considering with kepler community operator and kepler community helm chat as deployment approaches.

[ ] do we have issue open on kepler operator or kepler helm chat with TLS support?
[ ] are we going to integrate with cert mgr or sidecar as istio? by document supporting or we just leave it to user?
[ ] what if kepler running as a static pod?

and btw, any impacts to kepler RPM service?
from security point:
what's specific level for kepler's sensitive data? what are they?

Additionally, Kepler is handling sensitive data.

how to protect those data according to it's level? to clear/clarify our security boundary.

[ ] do we need considering a key/a cert with password?
[ ] do we recommended user to enable tls verify when enable tls?
[ ] do we need considering mtls?

for instance:
if we considering pod list is a sensitive data, and which should not listed by plaintext through network, as kubelet having TLS to protect those info. we should add TLS to protect those info as well. I suppose this is from node mgr's points of view.
if we don't want user A to see user B's pod on the kepler metric... should we considering username and password? I suppose this is from user's point of view.

from impl points of view:
is there any chance for us to reuse https://github.com/prometheus/exporter-toolkit
ref
https://github.com/prometheus/node_exporter/blob/d0c1d00d18867297326bc7b1c21de36ae50e6a92/README.mdplain=1#L383
reuse https://github.com/prometheus/exporter-toolkit/blob/master/docs/web-configuration.md
https://github.com/prometheus/exporter-toolkit/tree/master/web
https://github.com/prometheus/node_exporter/blob/d0c1d00d18867297326bc7b1c21de36ae50e6a92/go.mod#L28


Fix: Modify type declaration to DateTimeImmutable

This PR

[x] modifies a type declaration from DateTimeInterface to DateTimeImmutable

Thanks @localheinz

Thank you, @chartjes!


What's the bert_model in glyph_config

I am confused that what should I write in "bert_model" item.
When I use vocab.txt of bert pretraining model, error occurs.
When I use the path of bert pretraining model, error occurs too.
glyce/configs/glyce_bert.json

"glyph_config":{
         "hidden_dropout_prob": 0.5,
          "max_position_embeddings": 512,
          "bert_model": "/home/ubuntu/nieping/train_logs/models/bq",

thank you for following our work!
i suppose your problem is due to the improper settings of the config file, for which we will add more detailed instructions for this soon.
if you have any other problem, feel free to inform us!

thank you for following our work!
i suppose your problem is due to the improper settings of the config file, for which we will add more detailed instructions for this soon.
if you have any other problem, feel free to inform us!

It seems like no newer progress?

Could you please add more details to the error?
"bert_model" is the path to the folder that contains four files: [bert_config.json], [vocab.txt], [config.json], [pytorch_model.bin].
Or you can get the latest code by "git pull" and try again.


chore: remove console.log

Thanks for your patch :)
I'll be moving this repository to my company's organization (@wongnai) soon.


#50 Modify LambdaStep to use Resource as Lambda ARN

Issue #50
Modification of LambdaStep to accept parameter lambda_arn. If parameter exists it will use it to create Lambda step in format:
"Step_id": {
"Type":"Task",
"Resource":"lambd_arn"
}
as described in step function documentation - https://docs.aws.amazon.com/step-functions/latest/dg/connect-lambda.html
Unit tests passed.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

AWS CodeBuild CI Report

CodeBuild project: StepFunctionsPythonSDK-integtests
Commit ID: 7263fb73e45855db8365222230442b7f81079bda
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

As mentioned in #50, we would not want to update the LambdaStep for this change, I will close the PR.


move test data to resources

all tests are at the stsc.integration.tests project (https://github.com/sidorovis/stsc.integration.tests) they are moved to resources folder


Wrong license - Should be GPLv2

Esper is licensed GPLv2 and Apache 2 license is not compatible

@bernhardttom thanks, it's now corrected


Incomplete records read from Delta Table

Dear Team,
Few delta tables were created last year (23-Nov-2020) and records were getting merged on a daily basis. When the table was queried, we see that half of the partitions are missed to read by delta driver. We can see the partitioned folder, but these folders are not read by delta driver.
What could be the root cause? Highly appreciate your help.
Cluster used to run merge query: 7.3 LTS (includes Apache Spark 3.0.1, Scala 2.12)
When queried over partial version history, see no delete operation. Also the Merge operationMetrics do not have any deleted rows.
Thanks and Regards,
Vijay

Hi @VijaybabuNakkonda - this appears to be a Databricks-specific question, could you please ask your question in Delta Users Slack > databricks-delta channel or contact Databricks support.
Saying this, as these are long-running Delta tables, if you have been doing any overwrites, updates, deletes, or merges - it's possible that these partitions are only used for time travel purposes.

Hi @VijaybabuNakkonda please do raise this issue in the other channels so we can help.  But please note, when Delta Lake performs any non-append operations - such as overwrites, updates, or merges - new files added which allows for time travel and helps ensure consistency. For more on the topic, please refer to Unpacking the Transaction Log.
I'll close this issue per other channels but please re-open if appropriate.  Thanks!

Thanks for closing the issue @dennyglee
For someone facing same kind of issue. I found the potential root cause. When the delta transaction is failed merge operation in this case, old delta log versions are being deleted, causing the incomplete delta read operation.

Thanks for the update @VijaybabuNakkonda - could we find a time to chat via Slack later this week?  If this is true, then this is a potential bug.


SVD inconsistency for sparse matrix with LOBPCG

SVD inconsistency for sparse matrix with LOBPCG
Computing singular values using the solver LOBPCG, with a different number of singular values ask, is not stable in the sense that it produce different values.
To be more precise, take the sparse matrix:
import numpy as np
from scipy.sparse import diags
from scipy.sparse.linalg import svds

mat_size = 128
mat_diag = np.full(mat_size, -2)
mat_hors_diag = np.full(mat_size - 1, 1)
mat = diags((mat_hors_diag, mat_diag, mat_hors_diag), (-1, 0, 1))

Then, I will compute the 16 largest (LM) and smallest (SM) singular values of the matrix using either the solver arpack or lobpcg. I will do this two times by first asking for the 16 LM/SM and then for the 32 LM/SM singular values and compare the 16 LM/SM from these two computations. We should get that the difference between the two is close to machine precision.
First, as a reference, the 16 LM singular values with arpack:
>>> svd_16 = svds(mat, k=16, which="LM", return_singular_vectors=False, solver="arpack")
>>> svd_32 = svds(mat, k=32, which="LM", return_singular_vectors=False, solver="arpack")
>>> print(np.abs(svd_16-svd_32[-16:]).max())
3.552713678800501e-15

The maximum of the difference is close to machine precision, that is good.
Then the 16 LM singular values with lobpcg:
>>> svd_16 = svds(mat, k=16, which="LM", return_singular_vectors=False, solver="lobpcg")
>>> svd_32 = svds(mat, k=32, which="LM", return_singular_vectors=False, solver="lobpcg")
>>> print(np.abs(svd_16-svd_32[-16:]).max())
1.1664855303905597e-08

We have lost half the precision.
And the SM version with lobpcg:
>>> svd_16 = svds(mat, k=16, which="SM", return_singular_vectors=False, solver="lobpcg")
>>> svd_32 = svds(mat, k=32, which="SM", return_singular_vectors=False, solver="lobpcg")
>>> print(np.abs(svd_16-svd_32[:16]).max())
0.03882329852828505

It is even worse. I did not put the SM result with arpack as it does not converge in that case.
To track how this behaves with respect the matrix size, I made a plot of those maximum differences with respect to the matrix size (see svd_scipy.pdf). We can see that the LM Arpack stay close to machine precision that good. However, the LM/SM with LOBPCG curve seem good for small matrix size but at a matrix size of 80, they have a jump and a big one for the SM version.

Additional information:

This behavior does not seem to depend on the values in the matrix neither if they are real nor complex.
The matrix size for which the jump occurs depend on the number of singular values ask (here 16/32).
SciPy/NumPy/Python version information>>> import sys, scipy, numpy; print(scipy.__version__, numpy.__version__, sys.version_info)
1.7.3 1.21.4 sys.version_info(major=3, minor=9, micro=5, releaselevel='final', serial=0)

Code for the plot:
import matplotlib.pyplot as plt
import numpy as np
from scipy.sparse import diags
from scipy.sparse.linalg import svds

def matrix(mat_size):
    mat_diag = np.full(mat_size, -2)
    mat_hors_diag = np.full(mat_size - 1, 1)
    return diags((mat_hors_diag, mat_diag, mat_hors_diag), (-1, 0, 1))

def LM_Arpack(mat_size):
    mat = matrix(mat_size)
    svd_16 = svds(mat, k=16, which="LM", return_singular_vectors=False, solver="arpack")
    svd_32 = svds(mat, k=32, which="LM", return_singular_vectors=False, solver="arpack")
    return np.abs(svd_16 - svd_32[-16:]).max()

def LM_LOBPCG(mat_size):
    mat = matrix(mat_size)
    svd_16 = svds(mat, k=16, which="LM", return_singular_vectors=False, solver="lobpcg")
    svd_32 = svds(mat, k=32, which="LM", return_singular_vectors=False, solver="lobpcg")
    return np.abs(svd_16 - svd_32[-16:]).max()

def SM_LOBPCG(mat_size):
    mat = matrix(mat_size)
    svd_16 = svds(mat, k=16, which="SM", return_singular_vectors=False, solver="lobpcg")
    svd_32 = svds(mat, k=32, which="SM", return_singular_vectors=False, solver="lobpcg")
    return np.abs(svd_16 - svd_32[:16]).max()

N = np.arange(33, 129)
lm_arpack = [LM_Arpack(mat_size) for mat_size in N]
lm_lobpcg = [LM_LOBPCG(mat_size) for mat_size in N]
sm_lobpcg = [SM_LOBPCG(mat_size) for mat_size in N]

fig, ax = plt.subplots(constrained_layout=True)

ax.semilogy(N, lm_arpack, ".--", label="LM Arpack")
ax.semilogy(N, lm_lobpcg, "+--", label="LM LOBPCG")
ax.semilogy(N, sm_lobpcg, "x--", label="SM LOBPCG")

ax.grid(True)
ax.set_xlabel("matrix size", fontsize=15)
ax.set_ylabel("difference", fontsize=15)
ax.legend(loc=2, fontsize=15)

plt.show()

Thanks for raising the issue! Here are some explanations of the observed behavior:
For the smallest singular values, svds itself has problems that hopefully get resolved with https://github.com/scipy/scipy/pull/11829
For the largest singular values in your tests svds probably hits the undocumented inconsistency in the default values of maxit in arpack and lobpcg, as well as the default tolerance, which svds does not even allow to set and propagate to the solvers. The default  default tolerance in lobpcg is  n*sqrt(eps), which is much larger than the default in arpack that attempts to be the utmost accurate.
Another undocumented in svds feature of lobpcg is that for small matrix sizes lobpcg does not bother to run instead converting sparse matrices into dense and calling a dense solver which gives machine precision accuracy - that's why you see the jump in accuracy as you increase the matrix size.  It's documented in lobpcg docstring, but not in svds.
So testing lobpcg for small size matrices is meaningless. For large size matrices, you can try varying maxit, which is defaulted to 20 in https://github.com/scipy/scipy/blob/master/scipy/sparse/linalg/_eigen/lobpcg/lobpcg.py . That should probably resolve the big difference that you observe.

Thank you for the explanation. It does seem that increasing maxiter allows to recover the double precision for the LM case and at least go to single precision in the SM case.

Thanks for confirming. The single precision tolerance is the lobpcg default for double precision calculation, which is consistent with your results. Please keep the issue open to motivate at least improving the svds documentation.

From: Zoïs Moitier @.>
Sent: Wednesday, December 15, 2021 3:15:48 AM
To: scipy/scipy @.>
Cc: Knyazev, Andrew @.>; Comment @.>
Subject: Re: [scipy/scipy] SVD inconsistency for sparse matrix with LOBPCG (Issue #15170)
[External Email - Use Caution]
Thank you for the explanation. It does seem that increasing maxiter allows to recover the double precision for the LM case and at least go to single precision in the SM case.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHubhttps://github.com/scipy/scipy/issues/15170#issuecomment-994479220, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AKFMTPME6DRIWJ5VDTVFJX3URBFDJANCNFSM5JRPH7CA.

I am wandering why maxiter default is a constant, would it be better if it depend on the size of the matrix.

It's related to the distribution of the eigenvalues and the quality of the initial approximation, rather than the size of the matrix. It's chosen intentionally low, and the latest revision of lobpcg gives a warning if the chosen tolerance is not met, to attract attention of a user to its importance .

That would be a nice addition because I first try to increase the Krylov dimension but that did not works and I did not think about maxiter.

@zmoitier It appears that the issue has been resolved and could be closed?

Yes, it does. Thanks for letting me know.


revise fts.nav JS

what this PR accomplishes is the issue where aria-expanded was an attribute on the navigation when moving up from small screen to large screen views, where the navigation was no longer an expandable/collapsable object.
The revision does the following:
on resize up to large screen, check to see if the navigation had an aria expanded attr of true.  if so, apply a class of was-expanded to the navigation.
whether the navigation had an aria-expanded of true or false, it still needed to be removed when in desktop mode, so now the attribute is completely removed.
when moving down to small screen view, if the navigation has the class 'was-expanded' remove that class and reset the aria-expanded to true

made a revision to my revision because it needed to be revised some more.
it's better now.


Use unique_ptr for buffers

Another performance improvement is to use unique_ptr instead of shared_ptr for buffers, maybe worth to port as well, check this

Thanks for your suggestion. Actually we plan to make the implementation of buffer pluggable, as adding a few bytes of meta in front of user data is disruptive (for those who want to avoid memcpy).

Thanks for your suggestion. Actually we plan to make the implementation of buffer pluggable, as adding a few bytes of meta in front of user data is disruptive (for those who want to avoid memcpy).

May I know when would this feature plan to be added as original one really hurts performance? Or are there any suggestions to minimize memcpy issue?

@sheepgrass
Currently, we don't have ETA as it is supposed to be a significant overhaul of the code.
memcpy issue exists only when your system uses its own buffer structure and tries to put the data in the buffer to Raft using append_entries(...) API. In such a case, you should allocate a separate ptr<buffer>, do memcpy from your buffer to ptr<buffer>, and then call Raft API with ptr<buffer>, which is quite redundant.
One possible workaround is using ptr<buffer> for your system as well, then you can directly put data to Raft without memcpy call.

@greensky00
Thanks for your reply.  Actually, I came up with the workaround yesterday just similar to what you suggested.  What I want to mention is that I want to minimize dynamic memory allocations as many as possible but using the provided buffer class would force us to use alloc().  To workaround this, I need to 'hack' my own buffer to have structure same as the provided buffer.  I think it's a common scenario having data in a different structure other than NuRaft buffer.
And on a side note, if I got it right, calling append_entries() is a must when using NuRaft and I found that there are many dynamic memory allocations which I want to avoid.

'hack' my own buffer to have structure same as the provided buffer.
Yes, this is the same way as what we are doing internally here.

Maybe we can provide a new overloaded append_entries() function, which accepts the general memory pointer and its size as parameters, and reduces the number of internal memory allocations. It will be a much smaller work than supporting generic buffers.

Maybe we can provide a new overloaded append_entries() function, which accepts the general memory pointer and its size as parameters

Yes, it can help if such function can be provided. Beware that not to memcpy inside that append_entries() otherwise there is no point to have such function. (I think such approach is moving the 'hack' from outside of append_entries() to inside)
Also, be aware of the handling of log_entry inside append_entries() which may cause dynamic memory allocations and cause difficult to implement the new approach.


Document procedure for contributing to project

Formally list the project's dependencies using a requirements.txt file
and extend the documentation with set up information.

Might make it easier for people to contribute. :) Thank you!

No problem :)


Enable GitHub Action testing on Darwin and Windows

See https://github.com/senzing-garage/sz-sdk-go-core/issues/240

Pull requests:

https://github.com/senzing-garage/serve-grpc/pull/340


Object Names and IDs - Add section for qualified names

I was having a really rough time trying to figure out where the rules were defined for the type property of a PodCondition. I know underscores are allowed but I didn't find a rule in this part of the docs that mention that _ is allowed.
Turns out this is the code that validates that property,
https://github.com/kubernetes/kubernetes/blob/abc056843c030ead11551e13171a4447701db24e/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go#L42
(via https://github.com/kubernetes/kubernetes/blob/abc056843c030ead11551e13171a4447701db24e/pkg/apis/core/validation/validation.go#L5390)
Which has a really nice explanation of what a qualified name is right above it,
const qnameCharFmt string = "[A-Za-z0-9]"
const qnameExtCharFmt string = "[-A-Za-z0-9_.]"
const qualifiedNameFmt string = "(" + qnameCharFmt + qnameExtCharFmt + "*)?" + qnameCharFmt
const qualifiedNameErrMsg string = "must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character"
const qualifiedNameMaxLength int = 63

var qualifiedNameRegexp = regexp.MustCompile("^" + qualifiedNameFmt + "$")

It would be great if this information was included in this particular doc.

related page: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/

/language en

/sig docs

I think we should document this, but it belongs elsewhere. https://kubernetes.io/docs/concepts/overview/working-with-objects/names/ is about the name and uid fields within object metadata only. This issue is about object status.

@sftim is the "qualified name" used elsewhere? Or is it specific to object status type naming? If the latter I agree but if the former, it might be beneficial to stick it in this section.

I cannot think of a better location for this concept.

I'd put the explanation in https://kubernetes.io/docs/reference/glossary/ and transclude (eg {{< glossary_definition prepend="Kubernetes also has a concept of a qualified name, which is" term_id="qualified-name"  >}})) it into the Object Names and IDs page.


Fix dialogue box size on mobile devices

The dialogue box is small when viewed on mobile devices. My art (lake with a duck) has a rectangular layout, so I'm not sure on how it would work.
Contributions appreciated!

Hello, @burntcarrot. Can you assign me this issue?

@f1redr4g0n Sure!

Resolved via #5.


Pair the controller to the headset

I take this bug / request to congratulate you on your project.
Works really great!!!
It's a pleasure to enjoy  VR dolphin or skyrim VR unthetered and with the visula quality of the go!!!
Just I just need to pair the gamepad to the headset to being able of enjoying it everywhere.
Thank you!!!!!!

Thank you for comment.
Does "pair the controller" mean "connecting a gamepad on Oculus Go with bluetooth"?
I recommend that you connect a gamepad on PC.
If you connect a gamepad on oculus go, it introduce an additional input latency.

How do I play with a gamepad on the PC? I've got a steam controller connected but it doesn't work when in VR :(

Well after testing a lot: It works fine with xbox 360 controller but not with steam controller. wtf


🚀 Feature: Chat room rename functionality

🔖 Feature description
Presently, the inability to edit chat room names, which are automatically generated by the bot, can lead to difficulties in comprehending the chat room's content and recalling previous interactions with the bot. Therefore, implementing the chat room renaming feature has the potential to rectify these issues and improve overall clarity and understanding.
🎤 Why is this feature needed ?
The chat room renaming feature significantly enhances the user experience by providing a valuable means of contextualizing conversations within their respective chat rooms.
✌ How do you aim to achieve this?
Frontend:

I will add the small edit icon on the right side of the chat room name.

Backend:

I will add the new routes for the chat room rename, then, add the query changes in the respective routes.

🔄 Additional Information
No response
👀 Have you spent some time to check if this feature request has been raised before?

[X] I checked and didn't find similar issue

Are you willing to submit PR?
Yes I am willing to submit a PR!

Hi @dartpain
I want to work on this feature.
Kindly, assign this feature to me.

@tenokami can you support us with UI please?
Good idea
Thank you!

UI is ready @staticGuru
https://www.figma.com/file/OXLtrl1EAy885to6S69554/DocsGPT?type=design&node-id=0%3A1&mode=design&t=lSO86IRqK81Zuzpz-1

Thanks @tenokami .
You're UI design, exactly what I thought.
I will work on it.