add cache builder and loading cache

Codecov Report
Patch coverage: 53.50% and project coverage change: -6.98 :warning:

Comparison is base (73d3710) 95.01% compared to head (30d540b) 88.04%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more
Additional details and impacted files
@@            Coverage Diff             @@
##             main       #2      +/-   ##
==========================================
- Coverage   95.01%   88.04%   -6.98%
==========================================
  Files          11       12       +1
  Lines         963     1146     +183
==========================================
+ Hits          915     1009      +94
- Misses         33      109      +76
- Partials       15       28      +13

Impacted Files
Coverage Δ

internal/singleflight.go
33.94% <33.94%> (ø)

cache.go
72.00% <67.69%> (-16.89%)
:arrow_down:

internal/store.go
97.45% <100.00%> (+0.50%)
:arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.


PBL-22347 Add emulator control by browser

Open to further suggestions on how to improve the UI (and happy to port changes to CP if that makes sense)

This pulls in a lot of JavaScript; it might nice to cut it down a bit.

This looks good to me!
:+1:


SNPs still not showing mapping in download file

I did some analysis of the latest association downloads file (2023-10-29) according to the following steps

Filtered for associations with no data in  on CHR_ID column
Filtered on SNPS column for SNP IDs beginning with rs
Excluded SNPs containing " x " or ";" (multi-SNP associations)
Removed duplicate SNP IDs
This gave a list of 1624 potentially mappable SNP IDs
I ran these through the SNP validation tool implemented in DepoCuration
121 SNPs returned "Not found in Ensembl"
1503 SNPs can be found in Ensembl but do not have mapping in the download file
Manual check (vs Ensembl UI) of the first few suggests most of these have a patch location as well as canonical chromosome listed in Ensembl, but were not caught by the latest fix.

@sajo-ebi please investigate but we may need to wait for fix on Ensembl side (which should be implemented very soon)
Results of analysis are attached
SNP mapping analysis 7_11_23.xlsx

Unknown why these did not get mapped. Repeat analysis after next remapping.

I have reanalysed the data after the 20 Dec release. At step 8, 141 SNPs were found in Ensembl but do not have mapping in the download file. See attached file:
125 were newly added to the database in this release ("new SNP"). All those I checked had patch locations.
9 pre-existing SNPs had no mapping either in this release or the previous one analysed ("no mapping in previous"). Of these, 7 had patch locations and two did not (rs34420345 and rs781710751).
6 pre-existing SNPs had mapping in the previous release ("mapping was present before"). In all cases, new data had been added (new or updated associations for the SNP) since the last release, and now no mapping is shown for any of the associations. SNP mapping analysis 8 Jan 2024.txt

Since we know the issue with patch locations should be fixed in the next Ensembl release, I suggest we don't do anything about those, but it would be worth investigating the other issues i.e:
rs34420345 and rs781710751 have no mapping either in this release or the earlier one
These 6 SNPs had mapping previously, but now do not:

This needs to be confirmed after the data release of today is executed. @sajo-ebi

There are still many valid SNPs without mapping in the latest release (538 rsIDs). 469 of these had mapping in the previous release that I analysed (in Dec).
I’m struggling to see any pattern, a spot check suggests several are on patch locations but not all. I noticed there are batches from the same publication, for example 55 from PMID 34648354
( I haven’t been able to check if they are all in the same study). The list of unmapped valid SNPs is attached.  Unmapped valid SNPs Jan 2024.txt

@ljwh2 I analysed since we last ran mapping pipeline against the Missing SNP based on 20 Dec release , out of the 140 rsId there was only 1 rsId 'rs41302593'  which was unmapped in the new list based on the current data release . So apart from the missing Rsid execution , the remaining execution is just for scheduled mapping run when a new submission is made , the remaining Rsid were already unmapped in DB . I am not clear about what changes with every Data release that bring new unmapped rsid as only scheduled mapping is running so these rsids were already unmapped & picked up by latest data release . I believe this will be fixed when we do a full remapping again which will mean Full remapping executes against entire dataset not selective like happening now

Results from DR 2024-03-11:
There are 3236 potentially mappable rsIDs. Of these only 124 are valid in Ensembl but have no mapping in the Catalog.
I did some manual check of around 30 and nearly half have some issue in the Ensembl data.
For example these are found in Ensembl but have no mapping information:
https://www.ensembl.org/Homo_sapiens/Variation/Explore?v=rs691461;vdb=variation
rs527382443
rs691461
rs7031748
rs2437258
Some have no mapping to any canonical chromosome:
rs41302593
rs72617242
rs6004031
rs9406351
rs10423754
rs361433
Here are some examples that look fine in Ensembl
rs1854685
rs66767559
rs4027773
rs73598500
rs1795325153
Full list of 124 rsIDs attached, action for @sajo-ebi to rerun mapping against these and check if any more can be mapped. Unmapped SNPs March 14 2024.txt

@sajo-ebi hs triggered the mapping pipeline for the rsIds above, so @ljwh2 please check if those SNPs are now mapped.

@ljwh2  Attached it the association mapping report from the 124 rsids

@ljwh2 attached is the SNp to Rsid mapping ,I can see many chr ones; only thing I can think off is while the file based run was happening , the scheduled one was also running , hence some association apart from the list were also present in the report , there are no logs now to conclusively tell.    Asscn_Rsid_mapping.tsv.zip

Of the 124 SNPs:
58 got successfully mapped in the second round
53 exist in Ensembl with no mapping information
12 have only a patch location
One remains that could be mapped, this is rs1854685

@sajo-ebi to add better error logging. We need to be able to distinguish when Ensembl API returns blank response compared to SNP not found.

@sajo-ebi to add details of where this information can be found in the database

We can use the following query to check mapping errors in DB
select count(*) from association_report where snp_error is not null

Hi @ljwh2

can be closed


Chatkit: New avengers cannot join old rooms

error: "services/chatkit/authorization/cannot_join_private_room",
error_description: "Unable to join a private room. Users must be added to a private room by a member of the room",
error_uri: "https://docs.pusher.com/errors/services/chatkit/authorization/cannot_join_private_room"

Again, this is on the backend. The backend logic when creating a room is to add the student and all the teaching staff in.
However, if the teaching staff is not in the users list when the room is created, he wont have access to the room.


Add base section class

Codecov Report
Attention: 14 lines in your changes are missing coverage. Please review.

Files
Coverage Δ

src/pandasx/section/__init__.py
0.00% <0.00%> (ø)

src/pandasx/section/base.py
0.00% <0.00%> (ø)

:loudspeaker: Thoughts on this report? Let us know!.


Have a tickbox to backup current courses if they get overwrite

Backups current courses in a backup folder, if they get overwritten
Setting is on default true

Tickbox is there, no logic currently


First draft for openSSL-3 security advisory.

Signed-off-by: Kurt Garloff<EMAIL_ADDRESS>

We will probably need to finalize this and publish a v1 TODAY.
In order for this to happen, we need to:

Ensure our review of OSISM managed containers is complete (@berendt)
Ensure we have reviewed all containers that can be pulled in for the k8s cluster management (@garloff)
Make sure the wording is appropriate and correct (all)
Read the publication of the vuln, add links, CVE number, and summarize what this is for our advisory (technically skilled engineers can do this).

I think we're good for a v1, if @berendt confirms the correctness of the information on the OSISM side.
I'd like to push out at 18:30 CET, but @berendt and @fkr can veto this of course.
Otherwise an approving review would be helpful.

Please don't forget to create a symlink in the de subfolder. Otherwise, the article won't be displayed on the German website.

Please don't forget to create a symlink in the de subfolder. Otherwise, the article won't be displayed on the German website.

I would have overlooked this, thanks.
(Maybe we should have functionality to always link an English article when no German version is available?)
Approving review is required in order to merge this.

Thanks, Felix!


Add support for MemoryDB/ElasticCache/Redis as Idempotency backend

Updated topic to mention ElasticCache and general Redis as well.
https://aws.amazon.com/about-aws/whats-new/2021/08/amazon-memorydb-redis/

Hi all!
I was looking at some issues and opportunities to start contributing to Lambda PowerTools and came across this issue. This implementation looks very interesting and adding a new backend to persist Idempotency data seems like a great idea. Even DynamoDB is a great lightweight option, not everyone wants to create a new table and/or manage a new resource on AWS.
I'll try to summarize some information before I start writing code and tests.
1 - As everyone mentioned, MemoryDB is fully compatible with the Redis protocol and we can confirm that here https://docs.aws.amazon.com/memorydb/latest/devguide/memorydb-guide.pdf.pdf (page 1)
2 - @heitorlessa I'm not sure if the https://github.com/Grokzen/redis-py-cluster library is a good choice at this point. It might be a good choice when this thread starts, but now they suggest migrating to the official RedisLabs library (https://github.com/redis/redis-py)
3 - The official library supports connection and operations on a single node or cluster. https://github.com/redis/redis-py#cluster-mode
4 - Even the Idempotency utility will write and read few keys in Redis, a performance test is indicated to see if it can impact somewhere. I'll take care of that and share the results as I write the code.
5 - I create a new class to test how we can implement this feature and it is already saving and getting keys from Redis. Of course it's missing a lot of things like tests, parameters, code optimization, comments and other things, but I think it can be a start and I can start working on this.

6 - I will try to cover most scenarios in the first version.
Thanks for reading and suggestions are welcome.

Quick update: @Vandita2020 is working on this

Hey all! To give visibility to everyone of the work that @Vandita2020 and I are doing so that everyone can follow and give feedback.

For this implementation, we are using the official Redis Library - https://pypi.org/project/redis/

Vandita created a code base and together we are modifying it to make it ready to open PR.

Redis supports three types of architecture in its deployment: Standalone, Cluster and Sentinel. Each one has its characteristics and restrictions, which is not the case to explain more here, but it is extremely important for us to decide the types of connections we are going to support. Currently the code is ready to support Standalone and Cluster, adding Sentinel brings some complexity in how to initiate the connection and select the host, so we'll be planning to add this at the final.

Redis supports several types of keys, including a HASH type key. We chose the HASH type key because it supports a collection of field value pairs. This key was chosen due to the characteristics of the Idempotency utility, where we need to have fields such as: status, data_response, in_progress.

Redis supports the database index, which is a kind of namespace for information to be saved/stored between them. Our code already supports this.

Redis supports User ACL and we've added this support as well.

Below is a list of what we've done, what's missing and bugs. We already intend to open the PR as a draft this week so that everyone has access to the work done.

[x] Support for Standalone Redis

[x] Support for Redis Cluster

[ ] Support for Redis Sentinel

[x] User ACL support

[x] Database index support

[x] Add, update, get and delete idempotency record

[ ] Tests

[ ] Documentation

[ ] Convert TTL from timestamp to seconds

[ ] Review the get_record logic - It uses logic that only works with DynamoDB, but fails with everything else.

[ ] Improve docstring

We're making progress and hope to have this code ready for merge soon.

I've been writing code to connect to Redis Sentinel and it changes parameters and options a lot, so I thought of a new UX to make everything simpler. The way it is programmed now is:
from aws_lambda_powertools.utilities.idempotency import (
       idempotent,
       RedisCachePersistenceLayer,
       IdempotencyConfig
)

persistence_layer = RedisCachePersistenceLayer(host="<IP_ADDRESS>", port="6379", user="xxx", password="xxxx", db_index=0, static_pk_value="test",...)

The way this would be more readable to create a connection would be:
Standalone
from aws_lambda_powertools.utilities.idempotency.redis.connection import RedisStandalone
from aws_lambda_powertools.utilities.idempotency import RedisCachePersistenceLayer

conn_config = RedisStandalone(host.. pass.. user)
persistence_layer = RedisCachePersistenceLayer(connection=conn_config, ...)

Cluster
from aws_lambda_powertools.utilities.idempotency.redis.connection import RedisCluster
from aws_lambda_powertools.utilities.idempotency import RedisCachePersistenceLayer

conn_config = RedisCluster(host... startup_nodes)
persistence_layer = RedisCachePersistenceLayer(connection=conn_config, ...)

Sentinel
from aws_lambda_powertools.utilities.idempotency.redis.connection import RedisSentinel
from aws_lambda_powertools.utilities.idempotency import RedisCachePersistenceLayer

conn_config = RedisSentinel(sentinels..socket…)
persistence_layer = RedisCachePersistenceLayer(connection=conn_config, ....)

Makes sense? I would like to hear feedback on this.

We're reviewing the last edge case on concurrency and docs tomorrow - hoping to get this released next week

To enable other runtimes and customers to benefit from the research and implementation carried out in this pull request, I have provided the Redis implementation text below.
Many thanks to @roger-zhangg for the partnership and deep dive into this extensive and fantastic work, which undoubtedly raised the bar for this project!
API Design
We kept the same user experience when switching from DynamoDB to Redis. This is very important because one of the core principles of Powertools is to ensure the developer experience is as seamless as possible.
By keeping the persistence abstraction layer interchangeable between DynamoDB and Redis, we enabled a smooth transition that required minimal code changes for developers. The interfaces remained consistent, reducing friction when migrating the data storage technology.

Connection
In the initial version, we provided a dedicated Connection Class to assist our customers in creating Redis connections. This class wrapped the Redis client, both standalone and cluster, enabling customers to provide their Redis connection details (host, port, passwords) for establishing connections. Once the connection was established, this class could be passed to the Idempotency Layer for use. However, this design had a few significant drawbacks. Firstly, it was challenging for this design to support Redis sentinel connections, as sentinel connections are set up differently from standalone and cluster connections. Secondly, the default Redis client had more than 40 parameters. In our connection design, we chose to support only the most commonly used ones, passing all other parameters using **kwargs directly to the wrapped Redis Client. This could result in a less-than-optimal experience for our customers, as they would need to figure out the parameter names without IDE typing hints when passing parameters using **kwargs.
Keeping these considerations in mind, we drafted the second version of the Redis Connection. We made a few changes to the logic in the Idempotency Layer class so that it can now accept an established Redis client. With this design, customers can pass in any Redis client they prefer, as long as it adheres to the schema defined in the protocol class. Additionally, using the original Redis clients allows customers to leverage their prior experience with Redis and easily transfer and adapt their existing code. However, after some discussions, we concluded that this design may not be user-friendly for individuals without prior Redis experience. Therefore, we believe it's still beneficial to provide a helper class for creating connections to assist such users.
In the third and final design, we have opted to implement a helper connection class that assists customers in creating Redis connections with only the most commonly used Redis parameters (host, port, username, password, db_index, url, ssl). Simultaneously, we enable our customers to bring their own Redis connection if they prefer. One common use case, for example, is when customers want to use Redis with their certificates. This added flexibility allows them to establish secure Redis connections using their custom certificates while benefiting from the simplified connection setup provided by our helper class.
Example
from redis import Redis

from aws_lambda_powertools.utilities.idempotency.persistence.redis import (
    RedisCachePersistenceLayer,
)

client = Redis(
    host="host",
    port=6379,
    ssl=True,
    ssl_certfile=ssl_certfile,
    ssl_keyfile=ssl_keyfile,
    ssl_cert_reqs="required",
    ssl_ca_certs=ssl_ca_certs,
)

persistence_layer = RedisCachePersistenceLayer(client=client)

Orphan Records
Each idempotency record includes attributes such as expire_time, and inprogress_expire_time. These records should be automatically deleted when the current time reaches expire_time in the "completed" status or inprogress_expire_time in the "in_progress" status.
However, due to factors like Lambda handler timeouts, exceptions, or potential Redis expiration issues, there may be instances where idempotency records persist in Redis even after the current time has exceeded inprogress_expire_time while the record status is still "in_progress," or the current time has surpassed expire_time while the record status is "completed." We refer to these invalid records as "Orphan Records."
It's important to note that the method we implement to address these orphan records must be executed with caution to avoid potential race conditions. Further details on this issue will be elaborated upon in the following two paragraphs.
Redis HSET vs SET
In the idempotency workflow, we need to store idempotency records with multiple attributes in Redis. This can be achieved by using HSET with the idempotency key followed by attributes. Alternatively, we can encapsulate the idempotency key and all its attributes into a JSON format and employ SET to store the entire JSON structure.
In the initial design, we employed HSET to set idempotency records. HSET offers several advantages over SET: hash lookups are typically faster, and HSET allows us to set multiple attributes using the same hash key. This aligns perfectly with the requirements of idempotency records, as we need to store the idempotency key, expire_time, in_progress_expire_time, status, and payload under a single key. Thus, the utilization of HSET for storing idempotency records appears to be an optimal choice for this project.
However, there are two major drawbacks using this method.

Firstly, SET allows us to set an expiration time when creating the record, but HSET doesn't offer this capability. To set an expiration time, we must make an additional Redis call using EXPIRE for the respective key after using HSET. This could lead to reduced performance since it requires sending two Redis calls, significantly increasing latency due to the double Round Trip Time (RTT) to Redis. This also introduces the potential for orphan records. For example, if the Lambda handler times out immediately after creating the Idempotency record but before setting the expiration time, the record will become an orphan record as it won't automatically expire in Redis. One way to optimize this issue is to use Redis commands like a pipeline to combine HSET and EXPIRE into a single Redis request. However, this workaround also increases the complexity of the code.

Secondly, SET allows us to use the "nx" (non-exist) parameter to write only on keys that do not exist. However, HSET does not support this "nx" parameter. "nx" plays a crucial role in preventing race conditions, which will be explained in the following section on Redis Race Conditions.

During our experiments, we used pipelines with HSET to reduce the Round Trip Time (RTT). However, pipelines introduce complexity that can make code writing and testing more challenging:

Batch Logic - Pipelines do not execute commands immediately, requiring the inclusion of batch logic to group related operations and ensure they are executed together. This complexity adds to the application code.
Error Handling - Errors occur asynchronously after pipelined commands are sent, making error handling more challenging compared to synchronous execution.
Testing Complexity - Writing isolated unit tests that assert on command outcomes becomes more challenging, as test code must handle asynchronous results. Additionally, race conditions can occur more easily.
Debugging Challenges - In the event of a pipeline failure, it becomes more challenging to identify which specific command in the batch caused the issues. Extra logging must be implemented.
Transactional Integrity - Pipelines do not offer transactional integrity. If commands require transactions, additional logic is necessary to manually implement this on top of pipelining.
Compability - Some versions of Redis Cluster doesn't supports pipeline in transactional mode like pipe = r.pipeline(transaction=True)

Due to HSET lacking support for "EX" (expiration time) and "NX" (non-exist), and the challenges associated with pipelines, we have made the decision to transition to using SET in our final design.
Redis Race condition
There are two potential race conditions in the Redis Idempotency workflow. Although the probability of these race conditions occurring is low, if not addressed effectively, certain payloads may bypass the idempotency layer. This could lead to the underlying Lambda handler executing more than once for identical payloads, which is an undesirable scenario for our customers when utilizing our idempotency layer.
The first potential race condition occurs when two Lambda handlers simultaneously perform SET/HSET operations without using 'nx' on a non-existent record. Consider a scenario where, at the same time, two lambda handlers with the same payload both use GET or HGET to verify if a particular record is empty. While they both assume the record is empty, they both decide to write to the record using SET/HSET, resulting in the record being updated twice. Eventually, both Lambda handlers with the same payload pass the idempotency check and proceed to the execution stage. In this case, the idempotency layer would fails to prevent the handler from running twice on the same payload. See graph below.

This race condition can be resolved by adding nx=True when using SET. Once we apply nx=True to SET, even if two handlers execute SET at the exact same time, only one of them will succeed, and the other handler will recognize that the record exists and return accordingly.
The second potential race condition is similar to the first one but more complicated. This scenario occurs when two lambda handers are both trying to fix the same orphan record they found at the same time. In our idempotency workflow, if a Lambda handler encounters an existing idempotency record and identifies it as a corrupted or orphaned record, it proceeds to overwrite it with a valid record. However, in this case, we are attempting to overwrite a record in Redis, so we cannot use the nx=True flag in SET as we are modifying an existing record. Consequently, this scenario introduces the possibility of a race condition where two SET operations are executed simultaneously by two different Lambda handlers, potentially resulting in the record being updated twice, and both lambda handlers advancing to the execution stage. This is a situation that the idempotency mechanism should prevent. One solution for this is to use SET on a key with nx to serve as a lock, ensuring that only the handler that successfully acquires the lock proceeds to update the orphan record. See graph below showing each scenario:
without lock

with lock

Tests
We had challenges while writing tests to test the Redis interface and functionality, primarily because testing the Protocol without establishing a real connection is a little bit hard. Our initial approach was create an integration testing by running Redis locally in Docker containers, but spinning up container environments locally has some downsides, such as:

Slow - Launching containers locally introduced overhead and slowed down test execution.
Fragile - Tests became vulnerable to failures if containers didn't start correctly or if the local environment wasn't properly configured to support containers.
Resource intensive - Containers consume substantial amounts of RAM and CPU on developers' local machines.
Docker Registry Issues - Occasionally, downloading a container from Docker registries encountered failures.

Our solution was to shift towards more functional testing by injecting a fake "Redis client" class instead of using a real Redis connection. This approach offers the following advantages:

Speed - There is no spin-up time, resulting in significantly faster test execution.
Isolation - Tests concentrate on the application code logic without relying on external dependencies.
Lightweight - There's no requirement to install and configure containers on local machines.

Thanks


2.0 Extra Data on Character Event

Adds an extra_data shortcode parameter to Character events, to allow setting misc instructions on character Update events to give other directives to custom scene portraits, in a hopefully easier to use way than with a call_node to the custom portrait scene.  Since this isn't going to be used by most people, this is not added to the Timeline Editor, so is only exposed by the Text Editor or editing the timelines outside of editor.
Updates the PlainPortrait node with the functions for this parameter, and some brief documentation for how to use it when modifying PlainPortrait into a custom scene.

draft state while I figure out how to preserve it in Timeline Editor if it is set, without the parameter

seems to be saving just fine by timeline editor, so marking ready

Why is the extra data only passed on the Update mode not on Join mode?

Also as an alternative to making methods mandatory I would always suggest to use has_method() instead.
So instead of
if sprite.does_portrait_accept_extra_data():
	sprite.set_portrait_extra_data(extra_data)

you could do
if sprite.has_method('set_portrait_extra_data'):
    sprite.set_portrait_extra_data(extra_data)

This means one method less, especially a mandatory one.

So on why only Update and not Join, I'd be open to both, but its the way the functions are constructed. The Event calls the functions from the Subsystem, and a Join first calls a character add and then a character update after that. Therere's already a bunch of overloads to the function for optional parameters already, and I didn't want to add yet another one to the add first to then just pass to the update afterward. Thinking about it today, this particular event it might be better to actually pass a reference to the entire Event Object to the portrait subsystem instead, so it can just pull the values it needs from that
And for the second question, I actually didnt know/remember about the has_method() function. You're right, that would actually be a simpler way to do it, both for this one and the mirroring property too

alright I changed it to use has_method, and updated a couple other checks for portraits to use them too. revised the documentation for it on DefaultPortrait as well, since that's the template for a custom scene portrait.
and I just ended up adding the extra method to add_portrait after all as well rather than passing the reference to the entire event, since it would take  a lot more lines of rewriting to pull the data from the event instead, so its there on Join too now

Another reason not to do the "passing of an event" on the add_portrait method is, that at least in theory, the methods of the Subsystems are supposed to be usable from outside (so not from events).
That's the design idea at least, even though I'm not sure if it works everywhere.

ahh, yeah good point it would need to be usable in that way.  I guess i just dont like how there's so many parameters with defaults on one function, but i did change it to add the other one onto add after all . I guess the other alternate way would be splitting it into a whole bucnch of individual smaller functions and calling them in sequence


Fix review comment threads for pending reviews

Commit d6afb4f61dc4 ("gh_exec uses extra args like async_request")
inadvertently introduced some malformed GitHub CLI commands by merging
multiple field parameters together. This would result in the following
runtime error when attempting to add a comment to a pending PR review:
E5108: Error executing lua: .../gh.nvim/lua/litee/gh/ghcli/init.lua:834: attempt to call a string value
stack traceback:
        .../gh.nvim/lua/litee/gh/ghcli/init.lua:834: in function 'create_comment_review'
        .../gh.nvim/lua/litee/gh/pr/thread_buffer.lua:740: in function 'create'
        .../gh.nvim/lua/litee/gh/pr/thread_buffer.lua:842: in function <...ts/.vim/bundle/gh.nvim/lua/litee/gh/pr/thread_buffer.lua:823>

Fix it by introducing the necessary commas to separate the fields.
Fixes: d6afb4f61dc4 ("gh_exec uses extra args like async_request")
Fixes: #115

cc @m42e

❤


Version 0.5.1

Codecov Report
Attention: Patch coverage is 93.05556% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 91.09%. Comparing base (d9018b6) to head (707164f).
Report is 10 commits behind head on master.

:exclamation: Current head 707164f differs from pull request most recent head 1bdd12a. Consider uploading reports for the commit 1bdd12a to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #30      +/-   ##
==========================================
- Coverage   91.12%   91.09%   -0.04%
==========================================
  Files          11       11
  Lines        1003     1055      +52
==========================================
+ Hits          914      961      +47
- Misses         89       94       +5

Files
Coverage Δ

lyncs_utils/__init__.py
100.00% <100.00%> (ø)

lyncs_utils/pytest.py
94.23% <92.95%> (-3.85%)
:arrow_down:


Saving Video and Screenshots from Gazebo on Windows?

Gazebo 9 shipped with ROS Melodic on Linux has a working mp4 video capture button built in.
This is not the case on my Windows installation of ROS Melodic binary install obtained via Chocolatey. Clicking the button does nothing.

I've been using Windows game bar thing (Windows +G ) to capture videos, but it's an awkward extra step and I feel like the video quality isn't as good as a natively saved file.
I'm wondering if this is some relatively simple config thing that I can fix on my machine, like Gazebo is expecting to find some system installed ffmpeg or something?
The fact that the button doesn't respond makes it seem deeper than that, and now that I play around a little more it seems like the screenshot button crashes Gazebo as well.

Thank you for the report.
Can you tell us more about your work with ROS on Windows?

Can you tell us more about your work with ROS on Windows?

I'm working on developing a motion control library for a robot programming desktop software product. The product's cross-platform, but the main development track is Windows-centric, so I'm on Windows in Visual Studio most of the time.
At the moment, my main goal with ROS is plugging in our library to a simulated environment for algorithm testing. (Grabbing simulated or real sensor inputs, operating in a dynamic world, developing some robot controller mocking). We've started building our library on Linux as well, so Windows usage isn't as critical as when I started, but it's definitely convenient. Also potentially interested in interop between ROS and our product for customers that might want it.
By the way, please let me know if this kind of -desktop-full minor-functionality bug report from outside your team and in the realm of my use case is useful or not to the goals of the project here. I do greatly appreciate the efforts to make ROS native to Windows, and so far it's very minor issues like this that have come up!


Installing the software package is stuck

There is a problem with installing the software package：NPM 不是内部或外部命令，也不是可执行的程序或处理文件

Can you post the specific details of the error?

If you need assistance, please reopen.


Human readable serde implementation

This PR provides a human-readable serde implementation alongside the machine-readable one. Prior to this, human-readable (de)serialization just led to a todo!() panic.
Human-readable serde is used by certain serialization formats (such as ron), as opposed to machine-readable formats (like bincode). The main difference here is that human-readable serialization of Archetypes is row-wise, while machine-readable serialization of Archetypes is column-wise.

Before this can be submitted, a few things need to be done:

[ ] Rows should be serialized as tuples.
[ ] The column vectors need to be pre-allocated using Vec::with_capacity().

Codecov Report

Merging #12 (3367791) into master (01e9d70) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #12   +/-   ##
=======================================
  Coverage   59.93%   59.93%
=======================================
  Files           9        9
  Lines        1183     1183
=======================================
  Hits          709      709
  Misses        474      474

Continue to review full report at Codecov.

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

I have implemented serializing rows as tuples, along with serializing columns as tuples. Still need to properly allocate the Vecs.
In the future, it is possible to serialize Archetypes completely using tuples for both row- and column-wise serialization, thanks to the DeserializeSeed trait.

Changed the title to better reflect what this turned into. This was almost a full restructure of how serde traits are implemented. This fixes #1, and may possibly be the final form of (de)serialization, at least for a while.


Print backtrace on SIGILL & SIGSEGV

Use gdb to print backtrace in case we hit SIGILL or SIGSEGV. This uses gdb on systems where it is available.

In case gdb is not available:
Building project in /home/kll/terastream/acton/unwind
  Compiling unwind.act for development
   Already up to date, in    0.000 s
Building executable out/dev/bin/unwind
Hello World!

ERROR: segmentation fault
NOTE: this is likely a bug in acton, please report this at:
NOTE: https://github.com/actonlang/acton/issues/new
NOTE: include the backtrace printed below between -- 8< -- lines

-- 8< --------- BACKTRACE --------------------
Unable to get detailed backtrace using gdb or lldb
-- 8< --------- END BACKTRACE ----------------

ERROR: segmentation fault
NOTE: this is likely a bug in acton, please report this at:
NOTE: https://github.com/actonlang/acton/issues/new
NOTE: include the backtrace printed above between -- 8< -- lines
Aborted (core dumped)

@nordlander @aagapi @sydow what do you think about this? hit or shit?
I feel it's quite magical to call gdb but I like the functionality offered...

Okay, I'm going to proceed merging this one.


optimize_for_target_gateset OOMs on large CircuitOps

Description of the issue
When a circuit containing a large (in this case, 17 qubits & depth 20) CircuitOp is passed to optimize_for_target_gateset, it generates an OOM, possibly crashing the device. This circuit is well within the realm of "easy to simulate", so we shouldn't be OOMing or should at least warn users that this has a lower threshold than simulation.
How to reproduce the issue
qnum = 17
qubits = cirq.LineQubit.range(qnum)
subcircuit = cirq.FrozenCircuit(
    *[
      cirq.CX(qubits[i], qubits[i+1])
      for i in range(0, qnum-1, 2)
    ],
    *[
      cirq.CZ(qubits[i], qubits[i+1])
      for i in range(1, qnum-1, 2)
    ],
)
circuit = cirq.Circuit(
    cirq.CircuitOperation(subcircuit).repeat(10),
    cirq.measure(*qubits, key='out')
)
print(circuit)
# OOM: cannot allocate 256GB
supported_circuit = cirq.optimize_for_target_gateset(
    circuit,
    context=cirq.TransformerContext(deep=True),
    gateset=cirq.CZTargetGateset(),
)

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/transformers/transformer_api.py", line 380, in func_with_logging
    return _transform_and_log(
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/transformers/transformer_api.py", line 439, in _transform_and_log
    transformed_circuit = _run_transformer_on_circuit(
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/transformers/transformer_api.py", line 425, in _run_transformer_on_circuit
    return func(mutable_circuit if mutable_circuit else circuit, **kwargs)
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/transformers/optimize_for_target_gateset.py", line 143, in optimize_for_target_gateset
    circuit = transformer(circuit, context=context)
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/transformers/transformer_api.py", line 380, in func_with_logging
    return _transform_and_log(
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/transformers/transformer_api.py", line 439, in _transform_and_log
    transformed_circuit = _run_transformer_on_circuit(
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/transformers/transformer_api.py", line 425, in _run_transformer_on_circuit
    return func(mutable_circuit if mutable_circuit else circuit, **kwargs)
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/transformers/drop_negligible_operations.py", line 54, in drop_negligible_operations
    return transformer_primitives.map_operations(
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/transformers/transformer_primitives.py", line 143, in map_operations
    return map_moments(
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/transformers/transformer_primitives.py", line 87, in map_moments
    return _create_target_circuit_type(
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/transformers/transformer_primitives.py", line 43, in _create_target_circuit_type
    circuits.Circuit(ops)
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/circuits/circuit.py", line 1707, in __init__
    self.append(contents, strategy=strategy)
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/circuits/circuit.py", line 2254, in append
    self.insert(len(self._moments), moment_or_operation_tree, strategy)
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/circuits/circuit.py", line 1969, in insert
    moments_and_operations = list(
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/ops/op_tree.py", line 134, in flatten_to_ops_or_moments
    yield from flatten_to_ops_or_moments(subtree)
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/ops/op_tree.py", line 133, in flatten_to_ops_or_moments
    for subtree in root:
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/ops/op_tree.py", line 169, in <genexpr>
    return iter_transformation(
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/transformers/transformer_primitives.py", line 88, in <genexpr>
    (map_func(mutable_circuit[i], i) for i in range(len(mutable_circuit))), circuit
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/transformers/transformer_primitives.py", line 145, in <lambda>
    lambda m, i: [circuits.Moment(apply_map(op, i) for op in m.operations)],
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/circuits/moment.py", line 93, in __init__
    self._operations = tuple(op_tree.flatten_to_ops(contents))
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/ops/op_tree.py", line 113, in flatten_to_ops
    yield from flatten_to_ops(subtree)
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/ops/op_tree.py", line 112, in flatten_to_ops
    for subtree in root:
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/transformers/transformer_primitives.py", line 145, in <genexpr>
    lambda m, i: [circuits.Moment(apply_map(op, i) for op in m.operations)],
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/transformers/transformer_primitives.py", line 130, in apply_map
    c = circuits.FrozenCircuit(map_func(op, idx))
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/transformers/drop_negligible_operations.py", line 51, in map_func
    op if protocols.is_measurement(op) or protocols.trace_distance_bound(op) > atol else []
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/protocols/trace_distance_bound.py", line 73, in trace_distance_bound
    result = strat(val)
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/protocols/trace_distance_bound.py", line 98, in _strat_distance_from_unitary
    u = unitary_protocol.unitary(val, default=None)
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/protocols/unitary_protocol.py", line 119, in unitary
    result = strat(val)
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/protocols/unitary_protocol.py", line 183, in _strat_unitary_from_decompose
    state = qis.eye_tensor(val_qid_shape, dtype=np.complex128)
  File "~/git/cirq-realnoise-docs/cirq-core/cirq/qis/states.py", line 1076, in eye_tensor
    identity = np.eye(np.prod(half_shape, dtype=np.int64).item(), dtype=dtype)
  File "~/.virtualenvs/cirq/lib/python3.9/site-packages/numpy/lib/twodim_base.py", line 209, in eye
    m = zeros((N, M), dtype=dtype, order=order)
numpy.core._exceptions._ArrayMemoryError: Unable to allocate 256. GiB for an array with shape (131072, 131072) and data type complex128

Cirq version
0.15.0.dev

This happens because cirq.drop_negligible_operations(circuit) transformer calls cirq.trace_distance_bound(circuit_op) for the circuit operation, which in-turn tries to compute the unitary matrix of the circuit operation which results in OOM error.
qnum = 17
qubits = cirq.LineQubit.range(qnum)
subcircuit = cirq.FrozenCircuit(
    *[
      cirq.CX(qubits[i], qubits[i+1])
      for i in range(0, qnum-1, 2)
    ],
    *[
      cirq.CZ(qubits[i], qubits[i+1])
      for i in range(1, qnum-1, 2)
    ],
)
# Fails with OOM
cirq.unitary(cirq.CircuitOperation(subcircuit).repeat(10))

---------------------------------------------------------------------------
MemoryError                               Traceback (most recent call last)
 in
     11     ],
     12 )
---> 13 cirq.unitary(cirq.CircuitOperation(subcircuit).repeat(10))
~/quantum/Cirq/cirq-core/cirq/protocols/unitary_protocol.py in unitary(val, default)
117     ]
118     for strat in strats:
--> 119         result = strat(val)
120         if result is None:
121             break
~/quantum/Cirq/cirq-core/cirq/protocols/unitary_protocol.py in _strat_unitary_from_decompose(val)
181
182     # Apply sub-operations' unitary effects to an identity matrix.
--> 183     state = qis.eye_tensor(val_qid_shape, dtype=np.complex128)
184     buffer = np.empty_like(state)
185     result = apply_unitaries(
~/quantum/Cirq/cirq-core/cirq/qis/states.py in eye_tensor(half_shape, dtype)
1074         The created numpy array with shape half_shape + half_shape.
1075     """
-> 1076     identity = np.eye(np.prod(half_shape, dtype=np.int64).item(), dtype=dtype)
1077     identity.shape = half_shape * 2
1078     return identity
~/anaconda3/envs/cirq/lib/python3.9/site-packages/numpy/lib/twodim_base.py in eye(N, M, k, dtype, order, like)
212     if M is None:
213         M = N
--> 214     m = zeros((N, M), dtype=dtype, order=order)
215     if k >= M:
216         return m
MemoryError: Unable to allocate 256. GiB for an array with shape (131072, 131072) and data type complex128

To fix this, we can hardcode a number N s.t. we ignore the operation if it acts on > N qubits. The hardcode can exist in:

cirq.drop_negligible_operations transformer -- the cirq.trace_distance_bound and cirq.unitary will still fail if called on this operation.
cirq.trace_distance_bound method -- the cirq.unitary protocol will still fail
cirq.unitary -- I think this will be more invasive since we'd ideally expect unitary to work if running on a beefier machine where this memory allocation is possible.

My preference would be to simply change the cirq.drop_negligible_operations transformer and add a hardcoded check to ignore all operation acting on > 10 qubits, for example.
I'd be curious to hear what other maintainers think. We can bring this up in the next Cirq sync.

From cirq sync:
Add a special case to cirq.drop_negligible_operations and leave the other two methods as it is.


[RFC]: add fs/mkdir

Description
This RFC proposes adding the package @stdlib/fs/mkdir for creating a directory asynchronously and synchronously.
Package: @stdlib/fs/mkdir
var mkdir = require( '@stdlib/fs/mkdir' );
var mkdirSync = require( '@stdlib/fs/mkdir' ).sync;

Related Issues
None.
Questions
No.
Other
No.
Checklist

[X] I have read and understood the Code of Conduct.
[X] Searched for existing issues and pull requests.
[X] The issue name begins with RFC:.

Hi @kgryte , can I work on this?

@Daniel777y Yes, this would be useful. Note, however, that you'll want to accommodate older Node.js environments not supporting the options object (and hence recursive). You can consult https://github.com/isaacs/node-mkdirp, but note that the default API  for that package is now promise-based. You'll want to provide only a callback async API for fs/mkdir. Older versions of node-mkdirp were callback-based, but newer versions may have resolved some bugs. In short, you'll need to do some R&D to provide a polyfill for older Node.js versions.

@Daniel777y Yes, this would be useful. Note, however, that you'll want to accommodate older Node.js environments not supporting the options object (and hence recursive). You can consult https://github.com/isaacs/node-mkdirp, but note that the default API for that package is now promise-based. You'll want to provide only a callback async API for fs/mkdir. Older versions of node-mkdirp were callback-based, but newer versions may have resolved some bugs. In short, you'll need to do some R&D to provide a polyfill for older Node.js versions.

Hi, @kgryte , just to clarity, I only need to align with the latest usage but to accommodate older Node.js environments, right? For example, callback was optional in older versions, but not passing it will still throw an error in stdlib. Correct me if I am wrong.

Yes, I think that makes sense.


Identifiers validation failed for dataset Fichas diagnósticas para 10 especies de anfibios y reptiles mexicanos

Identifier validation failed for the dataset Fichas diagnósticas para 10 especies de anfibios y reptiles mexicanos:

Crawler attempt: 321
Publishing organization: Comisión nacional para el conocimiento y uso de la biodiversidad
Publishing country: MEXICO
Installation: IPTCONABIO
Number of occurrences indexed for this dataset at the time of the issue: 612
Cause: GBIF ID problems exceed 50% threshold: 100% duplicates; 575 total records; 575 absent records
Pipelines execution steps: http://api.gbif.org/v1/pipelines/history/execution/3740178/step

New IDs sample:

Old IDs sample:

06e2d6315d0f2a72cea87bfdb4a34856
0f2bbd3e46ac302e961006de137870e0
153588fdc8e97e58c5bad7391e976574
169e124a5cbc3b675d799fd3c19c4209
2412d9940f6e8a903485d3451e60bee5
3adf3125fed560972cfbb03b34dde997
46151fde95a3dc2ba326bf4b29ac3873
47db1e53673b395457a6feb44c482c78
4f5e06980fd02a1af11fdfc5181bd94e
4fab268c05702ad4c2a55fcfff1cdaa6

Publisher email
Hello,
I am contacting you from the GBIF Secretariat about a dataset published by the Fichas diagnósticas para 10 especies de anfibios y reptiles mexicanos : https://doi.org/10.15468/vikc14.
We noticed that the occurrenceIDs were changed. We have temporarily paused the ingestions of this dataset.
As you might already know, when an occurrence record has a new occurrenceID for a given dataset, our system considers it to be a new occurrence.
This means that it will be given a new gbifid and a new occurrence URL (like this one: https://www.gbif.org/occurrence/1252968762) and the old gbifid and URL will be deprecated.
In this case, this means that the occurrence URLs would be deprecated when ingesting the newest versions of these datasets.
We would like to check with you if those changes were intentional. Do you know if this is the case?
Please let us know, thanks! We are happy to resume the dataset ingestion.
Note that some users rely on those occurrence URLs and gbifids (like https://bionomia.net for example). In an attempt to improve
the stability of the occurrence URLs and gbifids, we have implemented a warning system to detect these type of changes in datasets (see this news item).
If the data publisher can provide us with a list of old and new occurrenceIDs per record, we can avoid the identifier and URL changes. Could that be an option?
Please let us know if you have any question. Thanks!
All the best,

You can skip/fix identifier validation using the registry UI.

Identifier validation still  failing for the dataset Fichas diagnósticas para 10 especies de anfibios y reptiles mexicanos:

Crawler attempt: 322
Publishing organization: Comisión nacional para el conocimiento y uso de la biodiversidad
Publishing country: MEXICO
Installation: IPTCONABIO
Number of occurrences indexed for this dataset at the time of the issue: 612
Cause: GBIF ID problems exceed 50% threshold: 100% duplicates; 575 total records; 575 absent records
Pipelines execution steps: http://api.gbif.org/v1/pipelines/history/execution/3741101/step

New IDs sample:

Old IDs sample:

06e2d6315d0f2a72cea87bfdb4a34856
0f2bbd3e46ac302e961006de137870e0
153588fdc8e97e58c5bad7391e976574
169e124a5cbc3b675d799fd3c19c4209
2412d9940f6e8a903485d3451e60bee5
3adf3125fed560972cfbb03b34dde997
46151fde95a3dc2ba326bf4b29ac3873
47db1e53673b395457a6feb44c482c78
4f5e06980fd02a1af11fdfc5181bd94e
4fab268c05702ad4c2a55fcfff1cdaa6

Publisher email
Hello,
I am contacting you from the GBIF Secretariat about a dataset published by the Fichas diagnósticas para 10 especies de anfibios y reptiles mexicanos : https://doi.org/10.15468/vikc14.
We noticed that the occurrenceIDs were changed. We have temporarily paused the ingestions of this dataset.
As you might already know, when an occurrence record has a new occurrenceID for a given dataset, our system considers it to be a new occurrence.
This means that it will be given a new gbifid and a new occurrence URL (like this one: https://www.gbif.org/occurrence/1252968762) and the old gbifid and URL will be deprecated.
In this case, this means that the occurrence URLs would be deprecated when ingesting the newest versions of these datasets.
We would like to check with you if those changes were intentional. Do you know if this is the case?
Please let us know, thanks! We are happy to resume the dataset ingestion.
Note that some users rely on those occurrence URLs and gbifids (like https://bionomia.net for example). In an attempt to improve
the stability of the occurrence URLs and gbifids, we have implemented a warning system to detect these type of changes in datasets (see this news item).
If the data publisher can provide us with a list of old and new occurrenceIDs per record, we can avoid the identifier and URL changes. Could that be an option?
Please let us know if you have any question. Thanks!
All the best,

You can skip/fix identifier validation using the registry UI.

Identifier validation still  failing for the dataset Fichas diagnósticas para 10 especies de anfibios y reptiles mexicanos:

Crawler attempt: 324
Publishing organization: Comisión nacional para el conocimiento y uso de la biodiversidad
Publishing country: MEXICO
Installation: IPTCONABIO
Number of occurrences indexed for this dataset at the time of the issue: 612
Cause: GBIF ID problems exceed 50% threshold: 100% duplicates; 575 total records; 575 absent records
Pipelines execution steps: http://api.gbif.org/v1/pipelines/history/execution/3741716/step

New IDs sample:

Old IDs sample:

06e2d6315d0f2a72cea87bfdb4a34856
0f2bbd3e46ac302e961006de137870e0
153588fdc8e97e58c5bad7391e976574
169e124a5cbc3b675d799fd3c19c4209
2412d9940f6e8a903485d3451e60bee5
3adf3125fed560972cfbb03b34dde997
46151fde95a3dc2ba326bf4b29ac3873
47db1e53673b395457a6feb44c482c78
4f5e06980fd02a1af11fdfc5181bd94e
4fab268c05702ad4c2a55fcfff1cdaa6

Publisher email
Hello,
I am contacting you from the GBIF Secretariat about a dataset published by the Fichas diagnósticas para 10 especies de anfibios y reptiles mexicanos : https://doi.org/10.15468/vikc14.
We noticed that the occurrenceIDs were changed. We have temporarily paused the ingestions of this dataset.
As you might already know, when an occurrence record has a new occurrenceID for a given dataset, our system considers it to be a new occurrence.
This means that it will be given a new gbifid and a new occurrence URL (like this one: https://www.gbif.org/occurrence/1252968762) and the old gbifid and URL will be deprecated.
In this case, this means that the occurrence URLs would be deprecated when ingesting the newest versions of these datasets.
We would like to check with you if those changes were intentional. Do you know if this is the case?
Please let us know, thanks! We are happy to resume the dataset ingestion.
Note that some users rely on those occurrence URLs and gbifids (like https://bionomia.net for example). In an attempt to improve
the stability of the occurrence URLs and gbifids, we have implemented a warning system to detect these type of changes in datasets (see this news item).
If the data publisher can provide us with a list of old and new occurrenceIDs per record, we can avoid the identifier and URL changes. Could that be an option?
Please let us know if you have any question. Thanks!
All the best,

You can skip/fix identifier validation using the registry UI.


How to use compileDynamicJump?

I was wondering what the use case for compileDynamicJump is, and what typescript syntax to use to trigger it?
I managed to trigger it with:
    switch((state)) {
        case 0:
            notify("Hi");
            break;
    }

But I assume this isn't the intended syntax?
If #44 goes ahead I would like to rewrite it to support the following syntax:
switch(state) {
  case value("c_battery", 1):
    notify("First case");
    break;
}

and remove the generation of the setNumber instruction

Yes, I was intending this to support numeric switch statements. And it uses a random "id" part of the value so that you can have multiple switch statements.
If we have value literals it could make sense to support them in a switch statement, but that could be kind of tricky. What if you want to have multiple switch statements with the same value as a case? I guess maybe we couple put them into separate subroutines?

What if you want to have multiple switch statements with the same value as a case

Good point. Better for users to use the label & jump instructions explicitly. Perhaps an inlined function (not a subroutine) could be the target of a jump. I'm looking for a clean syntax to implement a state machine pattern, and the numeric switch is good but I would prefer no additional instructions such as setNumber to be used.

What do you think about supporting this syntax?
export function main(state: Value) {
    label(value("c_battery", 1), () => {
        notify("Setup phase");
    });

    label(value("c_battery", 2), () => {
        notify("Loop phase");
    });

    jump(state);
    state = value("c_battery", 1);
}

I believe this should be safe w.r.t. local variable scoping as long as label is only permitted in the top-most block, though it would complicate any future variable usage optimizations.

That seems like a good way to handle labels. I assume you meant to put the jump after setting state?

Yes, correct. I'll follow up with a PR later. Thanks!


linebreaks incompatible with href using latex input and HTML-CSS output

When I turn on linebreaks: { automatic: true } then the links in my formula are not rendered.
When I turn off linebreaks, the links are rendered as expected.
Here follows an example of tex formula:
$$ \href(/audit/45, A) + \href(/audit/46, B) + \href(/audit/47, C) $$
I've reproduced with MathJax 2.3.0 and MathJax 2.4.0
Regards,
Lionel

I'm unable to reproduce this. Can you share a live sample (e.g., on jsbin, codepen etc) where this fails?
FWIW, the syntax is  $$ \href{url}{math} $$ (see the docs)

You can try with the following code. I reproduced it on jsbin: http://jsbin.com/lizanadiro/3/
In case you need it:
<head><script type="text/x-mathjax-config">
MathJax.Hub.Config({
  "HTML-CSS": {
    linebreaks: { automatic: true },
  }
  });
  </script>
  <script type="text/javascript"
  src="http://cdn.mathjax.org/mathjax/latest/MathJax.js?config=TeX-AMS-MML_HTMLorMML">
</script>
</head>
<body>
  <div style='width: 500px'>
  $$ \href{http://docs.mathjax.org/}{SomethingQuiteLongTooFillTheWholeScreen} + \href{http://docs.mathjax.org/}{SomethingQuiteLongTooFillTheWholeScreenToo} + \href{http://docs.mathjax.org/}{AnotherSomethingQuiteLongTooFillTheWholeScreen} + \href{http://docs.mathjax.org/}{And A LastSomethingQuiteLongTooFillTheWholeScreen} $$
  </div>
</body>

Thanks for following up. Strange; I could've sworn I did something similar yesterday and failed to reproduce. Oh well, I can reproduce it in your example.
Thanks so much for the report. We'll look into it.

The issue1022 branch of my fork of MathJax handles the hrefs during line breaks.

==> Merged.

==> In testsuite
MathMLToDisplay/Topics/LineBreak/issue1022.html


feat(llm): added the process of intelligent generated gremlin for retrieval before subgraph retrieval

added the process of intelligent generated gremlin retrivecal
added text2gremlin block in rag app

After implementing text2gremlin here, can gremlin_generate_web_demo.py  be removed?

After implementing text2gremlin here, can gremlin_generate_web_demo.py  be removed?

Yes, and it has been removed in new commit.

We should add a flag value under the interface of graph query rag/graph:

1 represents text2gql accurate matching success
0 represents generalization matching success
-1 represents no relevant information


docs: update repo_structure.mdx to reflect latest code changes

Description: This PR updates the documentation to reflect the recent code changes.

Thanks for this!


Android Auto: Nearby chargers loading forever

On my Fiat 200e (2020) the "nearby chargers" view does not show any charging stations, instead it keeps on loading.
After about a minute I closed it as it still did not load.

the photo was taken right next to a charging point
I till now have no experience with debugging on Android Auto.  But if you need some more information for debugging, let me know what you need and how I can obtain it (or a link to a guide) and I'll try my best.
Also huge thanks for developing and maintaining this great app! 👍

Thanks!
Have you tried the current beta version? It should be much better the changes in e182f8ae.

Hmm, 1.3.2 should already have those changes that I mentioned... However 1.3.3 does include some more changes to the Android Auto app which might have an influence. It has now been reviewed by Google, so should probably be available within a day or so.

had the same issues, which are now gone.

Yeah, sorry for not reporting back, I just forgot about it 🤦♂
Seems to also be fixed for me.


Interoperability Profile - session 7.3 and 7.4's example are incorrect

In “WACI-DIDComm Interop Profile” section 7.2 Presentation step 3.
The sequence diagram in section 6.2 Presentation describes this step is from Verifier to Wallet, but the example is:
  "from": "did:example:prover",
  "to": [
    "did:example:verifier"
  ],

The same as step 4.

It is reported and resolved here.
https://github.com/decentralized-identity/waci-didcomm/issues/10

thanks @echenknox ! we discussed on yesterday's call


Canonical Dimensions

The "COARDS" and by extension CF convention typically support XYZT axes. But we also have key use cases three (atleast) other canonical dimensions.

band, e.g. spectral or color.
ensemble member, e.g. monte-carlo simulation
scenario member, e.g. good, bad, ugly

How do we want to recognize (and support?) the existence of relatively common non spatio-temporal axes?
Do we want to consider any of these non spatio-temporal axes to be "canonical" in the sense that they are actually built into the convention and supported in general?

If WKT (or PROJJSON) is considered to express CRS, then the scope of which dimensions WKT/PROJJSON covers should be decided. Traditionnally what think about WKT CRS as being only about the "horizontal" coordinates, latitude/longitude, easting/northing. But WKT CRS allows also for compound CRS that are horizontal+vertical axis, or horizontal+temporal, or horizontal+parametric+temporal. Cf https://docs.ogc.org/is/18-010r7/18-010r7.html#125 and https://docs.opengeospatial.org/as/18-005r4/18-005r4.html#34
That said, such compound CRS involving temporal and parametric axis are quite exotic, and could be poorly supported by coordinate transformation libraries (although from a quick test transforming between a compound horizontal+parametric or horizontal+temporal to another horizontal CRS, I see the PROJ library behaves OK by just taking into account the horizontal component of the compound CRS)

Do we want to consider any of these non spatio-temporal axes to be "canonical" in the sense that they are actually built into the convention and supported in general?

To answer this, it would be helpful if you could elaborate what you mean by "supported". There are two flavors of possible interpretation:

"Supported" means that it's okay for Zarr arrays to have these other dimensions and still be valid GeoZarr, as long as the horizontal geospatial dimensions are clearly identified. This is relatively easy.
"Supported" means that parsing software explicitly recognizes the meaning of these dimensions and does something with them. If that's what we mean, I would urge that we only focus on fields that are relevant for geospatial position, such as the compound horizontal / vertical coordinates @rouault referred to, and exclude things like ensemble / scenario member which really have nothing to do with geospatial directly. Otherwise we will end up enumerating every possible dimension that climate data might have in the GeoZarr spec.

I think it is a given that we will have a scheme that accommodates dimensions not governed by the spec in the same way that CF and GDAL do. In the case of CF, you can add dimensions after XYZT or you can overload variable names with information that encodes an additional dimension into the data (both are done in the wild). In the case of geotiff, typical use of "band" is supported as a tiff image is inherently multiband in core use cases. But people overload "band" in practice as an additional dimension (like time or variable) or encode additional dimensions in file names.
Let's just zoom in on one important instance to clarify what I'm asking. What about "band"? I think a lot of people consider that core to interpretation of "geo" data.
Some points of reference to think about how use-case creep has been dealt with in other conventions:
In spatial geometry coordinates, we usually have XYZM, where M is a "measure" that can carry any number of additional quantities at the geometry node level.
In data cubes that follow COARDS/CF, we have XYZTABC where ABC would be additional dimensions that are not governed by convention.
In geotiff we have XYB where B is some extra dimension like time or band that is not strictly governed by convention but accommodated in practice as bands of a tiff image.
The real question is if we are going to try to recognize additional dimensions beyond XYZ at all?
CF, for example, doesn't support "band" as a dimension but geotiff (tiff really?) has various "band" accommodations. CF does support "time" as a dimension but geotiff does not.
So to summarize and clarify the question:
Let's assume XYZ are in scope for "geo".

Is time in scope?
Is band in scope?
Do we want to give informative (non-normative) guidance that might lead to enhanced interoperability for ensembles and scenarios?

CF, for example, doesn't support "band" as a dimension but geotiff (tiff really?) has various "band" accommodations. CF does support "time" as a dimension but geotiff does not.

CF defines that a variable may have any number of dimensions, and that dimensions other than those of space and time may be included.
Also CF defines standard names for bands.
a standard name for bands (e.g., sensor_band_identifier) , and thus allows defining dimensions of any type.

However, I also don't really catch the discussion comparing GDAL (tool) with CF (data conventions): when you convert source data (e.g., in GeoTiff, NetCDF, JP2K, etC.) to Zarr, the resulting metadata encoding in Zarr does not depend on the conversion tool (GDAL, Xarray, etc.) but on the source data format:

if you CRS is CF encoded in source, it remains CF encoded in target Zarr
if you CRS is WKT encoded in source, it remains WKT encoded in target Zarr.

Fair point on the "standard_name" accommodation for bands -- from a software implementers' point of view, that is akin to an overloaded variable name and more of a hack than what I would call "support". The point stands that there is no canonical dimension or in-built structural support for dimensions other than XYZT. While you can define additional dimensions, a software implementer has to divine the semantic meaning of the additional dimensions outside the scope of the convention.
I shouldn't have used "GDAL" in that example... Really, it should be "2D raster as typically supported by GDAL".
With regards to coordinate reference systems, that is not really core to the question posed in this issue, but it is a useful factor to the decision. CF does not lump time into its "grid_mapping" paradigm, favoring attributes carried on a time variable. As @rouault points out, in WKT, time and parametric coordinates are only used in exotic use cases.
This discussion really points to the answer being that:

Like CF and geotiff, geozarr will not have direct semantic support for dimensions beyond the traditional spatio-temporal XYZT but will have support for additional dimensions as generic additional dimensions.

Does anyone disagree with that and think the specification should, for instance, define a "band" or "ensemble" dimension and how to specify which dimension of a data variable is one of those?

While you can define additional dimensions, a software implementer has to divine the semantic meaning of the additional dimensions outside the scope of the convention".

I'm not entirely clear on your perspective (from my understanding, CF specifically supports XYZT, but standard names enable the unambiguous identification of the semantics of any other unit).
The significant advantage of using standard names for all GeoZarr variables is that it accommodates the description of most possible units used in dimensions, including wavelength, latitude, latitude_grid, sensor_band_identifier (and all units of measure for observed data), allowing users or clients to comprehend and process the data. This aspect is crucial for GeoZarr since we already have COG for 2D raster, and GeoZarr introduces the capability to support complex n-D arrays and ARD based on additional dimensions.

@dblodgett-usgs : I think I see your point if you mean the client doesn't know the corresponding wavelength for a specific band idnetifier. I suppose my example is not the best one.
Let me revise my example with our number one use case in HDSA project: for hyperspectral data, the wavelength can be indicated in dimension [sensor_band_central_radiation_wavelength] for example.
Any client supporting such kind of hyperspectral data will be able to discover which coordinates maps to the wavelength and be able to process the data acros its wavelength and the other dimensions.
(it might be a solution to address universaly bands if instead of the identifier, you always encode the corresponding wavelength, then it would become very standard)

My point there is that if additional dimensions are used to store band or ensemble, you don't know which dimension is which

If I understand your point differently, then key difference between NetCDF (CF) and GeoZarr is that all CF arrays have dimension names while Zarr arrays do not.
GeoZarr follows xarray conventions which define a special GeoZarr array attribute: _ARRAY_DIMENSIONS. The value of this attribute is a list of dimension names (strings), for example ["time", "lon", "lat"]
Therefore, you can discover as many dimensions as you wish.

@dblodgett-usgs : Ok, good approach.
When I talk about standard names, I'm refering the canonical units defined in the CF Standard Name Table which standardizes the identifier of most known unit: https://cfconventions.org/Data/cf-standard-names/current/build/cf-standard-name-table.html
We encoded multispectral PRISMA data product (not bands, but wavelengths) as follows in GeoZarr. Using the standard_name attribute, the client can interpret the various dimensions without ambiguity.
Dimensions:      (latitude: 1220, longitude: 1244, wavelength: 234)
Coordinates variables:
  float64 longitude(longitude) ;
    longitude:units = "degrees_east";
    longitude:standard_name = "longitude"
  float64 latitude(latitude) ;
    latitude:units = "degree_north" ;
    latitude:standard_name = "latitude"
  float64 wavelength(wavelength);
    wavelength:units = "m" ;
    wavelength:standard_name = "radiation_wavelength"
Data variables:
  float32 reflectance  (wavelength, latitude, longitude):
    reflectance  :units = "1" ;
    reflectance  :standard_name = "surface_bidirectional_reflectance"

Same example for the timeseries of Sentinel-2 data (with bands in this case)
Dimensions:      (band: 12, time: 739, y: 5490, x: 5490)
Coordinates:
  datatime64  time(time):
  float64 x(longitude) ;
  float64 y(latitude) ;
  float64 band(band);
    band:standard_name = "sensor_band_identifier"
(values:     (band) <'AOT' 'B02' 'B03' 'B04' ... 'B12' 'B8A' 'SCL' 'WVP'>
Data variables:
- float32 data(time, band, y, x)):
    data:units = "1" ;
    data:standard_name = "surface_bidirectional_reflectance"

Therefore, the client (of course adapted for hyperspectral data or multispectral data) can read the standard name attribute to interpret the semantic of the various dimensions.

OK -- thanks for that clarification.
So GeoZARR should support canonical dimensions for Wavelength/Band via coordinate variable standard names, "radiation_wavelength" and "sensor_band_identifier"?
What happens in the case of an auxiliary coordinate variable where the name of the variable is different than the dimension name?
There are two additional accommodations in CF for XYZT, the axis attribute and the coordinates attribute. In the case below, how would a client know that the wavelength variable is a coordinate for the band dimension of the x variable? I think we would use the coordinates attribute as I've shown below? Note that since time, lon, and lat are not auxiliary coordinate variables (they are COARDS style coordinate variables) we do not add them to the coordinates attribute. Note though that this seems to go beyond the intent of CF... quoting from the section linked just above:

Any longitude, latitude, vertical or time coordinate which depends on more than one spatiotemporal dimension must be identified by the coordinates attribute of the data variable. The value of the coordinates attribute is a blank separated list of the names of auxiliary coordinate variables.

This is the source of my statements that:

The point stands that there is no canonical dimension or in-built structural support for dimensions other than XYZT.

I like the idea of a proposed extension of band and perhaps some other non-spatiotemporal dimensions like ensemble member where we would point to auxiliary coordinate variables in a coordinates attribute and introduce addditional valid values for the axis attribute. e.g. XYZTB... for cases where you have more than one coordinate variable in the file and you want to express which is the primary axis to be used. (Axis specification is here.)
dimensions:
  lat = 180 ;
  lon = 360 ;
  band = 10;
  time = UNLIMITED ;
variables:
  double x(band,time,lat,lon);
    x:coordinates = "wavelength"
  double time(time);
    time:units = "hours since 1999-01-01 00:00" ;
  double lon(lon) ;
    lon:units = "degrees_east";
  double lat(lat) ;
    lat:units = "degrees_north" ;
  double wavelength(band);
    band:standard_name = "radiation_wavelength";

So GeoZARR should support canonical dimensions for Wavelength/Band via coordinate variable standard names, "radiation_wavelength" and "sensor_band_identifier"?

GeoZarr should support ANY standard name for the dimension coordinates of a data variable.
The currently used convention (which doesn't care about CF) is:

_ARRAY_DIMENSIONS: ordered list of dimension names (strings), for example ["time", "lon", "lat", "wavelength"]
for each defined dimension name: corresponding variable definition indicating the standard name (= semantic)

Moreover, going into details, I think that a ZEP-4 recommendation should provide a set of recommended standard names for various popular product types (e.g. multipspectral, hyperspectral, multi-tiles cubes, etc.)

It would be very useful to pull out list of standard_names that are used to indicate a variable is to be interpreted as a coordinate variable of a specific type.
What about my questions about  the coordinates and axis attribute detail that I call out above --- I don't see a response to those questions.
I'm not quite following:

currently used convention

What currently used convention?

for each defined dimension name: corresponding variable definition indicating the standard name (= semantic)

Does the variable name have to be the same as the dimension name? What if you had multiple variables that could be interpreted as coordinate variables?

What currently used convention?
The conventions described by current GeoZarr draft (partially based on xarray conventions).

It would be very useful to pull out list of standard_names that are used to indicate a variable is to be interpreted as a coordinate variable of a specific type.
GeoZarr/Xarray convention is that variable is declared in dimension is a coordinate (see xarray: https://docs.xarray.dev/en/stable/user-guide/terminology.html )

Does the variable name have to be the same as the dimension name? What if you had multiple variables that could be interpreted as coordinate variables?

Yes, for each dimension name listed in the data, you have Geozarr Coordinates (see classes defined in https://github.com/zarr-developers/geozarr-spec/blob/main/geozarr-spec.md)

What about my questions about the coordinates and axis attribute detail that I call out above --- I don't see a response to those questions.
Sorry, I have probably not understood the actual question.

OK -- so, we need to start linking to lines in the specification. I've been missing what's already there because we haven't been talking about specific text. Apologies.
So, according to: https://github.com/zarr-developers/geozarr-spec/blob/main/geozarr-spec.md?plain=1#L66
The geozarr "spec" (convention) incorporates XYZTBW already. So Band and Wavelength are canonical in the sense that a geozarr compliant software implementation would know how to work with data according to those dimensions.
With regards to coordinates and axis -- these two attributes do not appear in the current geozarr "spec" (convention). coordinats is used to connect a data variable to the appropriate coordinate variables for it in the case that the coordinate variable does not share a name with the dimension it describes. The axis attribute is similar to coordinates but is carried by the coordinate variable its self and indicates what canonical axis (XYZT currently in CF) a coordinate variable is intended to describe.

The geozarr convention (and xarray) incorporates every possible standard name as dimension which is something I really like --> this doesn't restrict you to encode rasters. In various mission, we have many arrays for which dimensions are not XYZT at all, and this provides a wide range of data to be encoded.
I would expect OGC-equivalent of "requirements-class" to potentially restrict/declare the typical allowed standard names for a set of usual domain (as you suggest: wavelength, altitude, band for satellite products).

From a data consumer's point of view, no restriction on what can be a coordinate of data is daunting. I understand how it is nice for a data provider, but we need to be careful not to create something that is nearly unsupportable.
OK, so getting back to the point of this issue:
geozarr will support XYZTBW. Where B is band and W is wavelength.
geozarr will not support Ensemble or Senario dimensions except as generic dimensions that must be interpreted manually or treated as semantically ambiguous in client software that does not recognize them.
Is that agreeable in terms of intended scope?

While I understand your perspective, I believe that making a decision at this stage would be premature without a more comprehensive view of the proposed specification or convention.
It's important to recognize that space data extends beyond raster XYZT data and often includes auxiliary data with varying coordinate systems. Our customers plan to convert data from numerous missions into the Zarr format, and as such, Geozarr is not simply another 2D raster container like GeoTiff.
On my side, I'm definitely against such restriction but again, would appreciate a convention based on profiles / requirements-class as in various OGC specs.

I don't think I'm being clear. It is very important that we fully understand the scope of this work up front.
It is a given that geozarr will allow n-dimensional variables with coordinate variables for dimensions other than XYZT that may not be a-priori known according to the convention. I think that is agreed? i.e. we will not limit what is allowable in terms of dimensions people want to include.
It is not a given that we will recognize dimensions that should be treated in a particular way in software. e.g. an "ensemble" dimension needs to be treated very differently from a "band" dimension in the use cases that are appropriate to those kinds of dimensions. A client software developer would benefit from consistent handling of these extra dimensions' metadata in geozarr so we don't end up in the place the NetCDF-CF is where some people put extra dimensions in variable names and others make custom dimensions that most software fails to represent.
You say:

space data extends beyond raster XYZT data and often includes auxiliary data with varying coordinate systems

Noting that there is no intention to limit inclusion of a-priori unknown dimensions types, the real question I'm asking is can and/or should we enumerate what some of those additional dimensions are so that the semantics of the data is known in the convention?

Thank you for the clarification, I better see the intent.

Noting that there is no intention to limit inclusion of a-priori unknown dimensions types, the real question I'm asking is can and/or should we enumerate what some of those additional dimensions are so that the semantics of the data is known in the convention?
Surely yes. Probably that in the upcoming months, we will better identify more representative use cases with related data type. So there might be core set of dimensions (XYZTBW ?) and additional recommendatiosn for a particular type of data (e.g. satellite, hyperspectral).

Great. I'm all for the typical remote sensing additional dimensions as being in scope for a "Geo" convention. What about ensemble and scenario? Those are modelling-specific concepts that are common in spatially referenced modeling data, but are they sufficiently core that we would specify how they should be referenced in GeoZarr?
My gut says yes with the caveat that it would need to be a very light hand approach -- e.g. via standard_name and perhaps use of a "coordinates" attribute for auxiliary coordinate variables.

On my side, I'm not familiar about those concepts, but why not, this is probably very relevant.

In terms of identifying representative use cases, I found the Sentinel-2 example stated earlier to be unexpected:

Same example for the timeseries of Sentinel-2 data (with bands in this case)
Dimensions:      (band: 12, time: 739, y: 5490, x: 5490)

Sentinel-2 image bands have three different pixel grids (10m, 20m, 60m) so I would not expect them all to have the same y and x dimensions.  @christophenoel does this assume that all bands had been resampled to a consistent (~20m) pixel size? If so, how would a multi-band image containing bands with differing pixel sizes be encoded?
(I am intentionally not using the term "spatial resolution" for the pixel grid spacing, even though the Sentinel-2 docs use that term.)

@tylere: Thank you for your insightful comment! Your suggestion is in line with my desire to incorporate data-specific recommendations in future iterations (multispectral data for example).
During the HDSA project, our primary objective was to make Sentinel-2 data more "processing-friendly" while avoiding the creation of separate Zarr files for each resolution (pixel grid).  As the conversion to Zarr involves costs, the aim is to maximize the efficiency of the target data by implementing additional, yet sustainable, encoding adjustments.
The consortium explored various approaches, such as defining subgroups or multiple arrays. In the end, we decided to downsample bands 2, 3, and 4, and upsample bands 1, 9, and 10.
Due to time and budget constraints, we weren't able to fully develop the proof-of-concept. However, the ultimate aim is to offer all bands across all three pixel grid sizes through multiscale functionality, always using the best available resolution.

I think this discussion is mostly tapped out. We don't have strong consensus other than, we will largely support what's in CF where standard_names imply that a coordinate variable should be interpreted as a given canonical dimension.


Adding squel statements with map()

Hi, I'm trying to dynamically build a squel chain. I have as input:
var model = {     'cube': 'CAP',     'measures': [       {         'field': 'AMOUNT',         'aggr': 'SUM',         'as': 'amount'       },       {         'field': 'OVERDUE_DAYS',         'aggr': 'SUM',         'as': 'days_overdue'       }   ] }
I would like to add them like this:
` var sql = squel.select();
model.measures.map(function(measure) {
  sql.field(measure.field, measure.as);
});

sql.from(model.cube)
  .toString();`

Is this at all possible?

Yes this is possible. See: https://jsfiddle.net/pberden/0ms4eugg/1/
Note to myself; stop working after 22.00 :sleeping:


[SCSS] AB Adding Additional Spaces into Quoted Text String

Description
SCSS adds additional spaces in a quoted string, breaking the code.
Input Before Beautification
This is what the code looked like before:
.class-name {
    color: blue;
    @include media('>=' + $switch-to-mobile-width) {
        color: red;
    }
}

Expected Output
The beautified code should have looked like this:
.class-name {
    color: blue;

    @include media ('>=' + $switch-to-mobile-width) {
        color: red;
    }
}

Actual Output
The beautified code actually looked like this:
.class-name {
    color: blue;

    @include media (' > =' + $switch-to-mobile-width) {
        color: red;
    }
}

Steps to Reproduce

Add code to Atom editor
Run command Atom Beautify: Beautify Editor
This beautified code does not look right!

Debug
Here is a link to the debug.md Gist: https://gist.github.com/f8f325d694eeb03601596957ec6c6ae3
Checklist
I have:

[X ] Tried uninstalling and reinstalling Atom Beautify to ensure it installed properly
[X] Reloaded (or restarted) Atom to ensure it is not a caching issue
[X] Searched through existing Atom Beautify Issues at https://github.com/Glavin001/atom-beautify/issues
so I know this is not a duplicate issue
[X] Filled out the Input, Expected, and Actual sections above or have edited/removed them in a way that fully describes the issue.
[X] Generated debugging information by executing Atom Beautify: Help Debug Editor command in Atom and added link for debug.md Gist to this issue

This is a defect present in Pretty Diff version 1.16.37 which is included with Atom Beautify.  I cannot reproduce the defect in the current version 2.1.16.  I am working on getting an updated version into Atom Beautify at https://github.com/Glavin001/atom-beautify/issues/881

@prettydiff Thanks!


GCS plugins filesystem

@mihaimaruseac
This is an initial implementation for gcs plugins. Unlike the s3 implementation, I wrote this implementation with the minimum requests made to server ( one request / one function, no request to pre-check if path is invaild, etc ). I have also included a log to the files changed when I run the test test.log here so we can review and discuss easier. ( it will be dropped later )

@mihaimaruseac
In additional, I think we should have some dicussions about my schedule for GSoC. Writing plugins seems does not take much time as expect. But I haven't thought about what will do after finishing writing plugins yet.

I'm thinking that each plugin would take around a week to fully land, so that would cover 2-3 weeks of GSoC (we're doing S3 and GCS). There will probably be a need for some API changes in the plugin interface (to solve issues discovered in these tests), so that could take 1-2 more weeks. Then, we'll have to move them to SIG IO and ensure that TF can still use them, on Linux, windows and mac.
My ideal plan is to have Windows and POSIX be the only plugins built by default in the pip package on the 2.3 release (~ midterm eval) and allow all other plugins to be loaded by the 2.4 release (~end of GSoC)

My ideal plan is to have Windows and POSIX be the only plugins built by default in the pip package on the 2.3 release (~ midterm eval) and allow all other plugins to be loaded by the 2.4 release (~end of GSoC)

It sounds good. Actually, I have some small ideas in mind related to filesystem ( I will try it when we finish the plugins ). They are implementing save model to cloud for keras.model ( not so hard ) and try fix some linking issue with the build on windows ( and static build in general )

I would like to ask an off-topic questions. Regarding the build on windows, it seems likely that we will face the issue about filesystem on windows again. After this PR #39124, tf.io works with s3:// but tf.io.gfile.GFile throws unimplemented. I think this is because s3:// is linked into pywrap_tensorflow_internal but not pywrap_file_io ( this is the internal implementation of GFile ).
If we loaded the plugin into tensorflow on Windows, I think that it will be linked into pywrap_tensorflow_internal but not pywrap_file_ioand the issue will happen again. I am trying to build a gcs_filesystem.dll on windows and load it with tf.load_library(). I will tell you what happen later.
My question is why we have 2 different parts for I/O, namely, tf.io and tf.python.lib.io. Is it better that we merge 2 parts into 1, let's say into tf.python.lib.io ? Merging into 1 part maybe the only solution for the issue we have on Windows ( As the code for registration filesystem will be only linked into 1, so we will have to register only one time )

I would like to ask an off-topic questions. Regarding the build on windows, it seems likely that we will face the issue about filesystem on windows again. After this PR #39124, tf.io works with s3:// but tf.io.gfile.GFile throws unimplemented. I think since there are multiple instances of Env::Default() instead of one shared instance on Windows, s3:// is registered into the Env::Default() of pywrap_tensorflow_internal but not pywrap_file_io ( this is the internal implementation of GFile ).

This looks like a bug for us that we will have to fix. I'll try looking into it. There should be only one filesystem registry, especially after the filesystem support is all converted to modular.

If we loaded the plugin into tensorflow on Windows, I think that it will be registered into the Env::Default() of pywrap_tensorflow_internal but not pywrap_file_ioand the issue will happen again. I am trying to build a gcs_filesystem.dll on windows and load it with tf.load_library(). I will tell you what happen later.

The ideal plan is to have this working in the modular filesystem world. If it doesn't, we need to fix. Let me know what happens.

Update: gcs_filesystem.dll was built successfully on Windows. The test result was the same as on Linux. tf.load_library() loaded successfully but both tf.io and tf.python.lib.io.file_io threw unimplemented. Maybe we need a different way to load the plugins ?

Oh, so there is a bug that we'll have to check. I'll be looking on this next week, after I unblock you with the TranslateName change.

My question is why we have 2 different parts for I/O, namely, tf.io and tf.python.lib.io.file_io. Is it better that we merge 2 parts into 1, let's say into tf.python.lib.io ? Merging into 1 part maybe a solution for the issue we have on Windows ( As there will be only one instance of Env::Default() left)

I think this is due to how the interface evolved, but will have to check. If we can reduce API that's also a great gain. I'll be back on this.

Another solution is build tensorflow on windows dynamically but there is a issue of protobuf on windows
https://github.com/protocolbuffers/protobuf/blob/master/cmake/README.md#dlls-vs-static-linking

The goal is to have all filesystem plugins be built dynamically. Due to the issues reported there on Windows, we have to have the memory allocation and deallocation routines included in the plugin.

My ideal plan is to have Windows and POSIX be the only plugins built by default in the pip package on the 2.3 release (~ midterm eval) and allow all other plugins to be loaded by the 2.4 release (~end of GSoC)

It sounds good. Actually, I have some small ideas in mind related to filesystem ( I will try it when we finish the plugins ). They are implementing save model to cloud for keras.model ( not so hard ) and trying to fix some linking issues with the build on windows ( and static build in general )

This is great and sure, we can do this as part of GSoC. Thank you.

Update: gcs_filesystem.dll was built successfully on Windows. The test result was the same as on Linux. tf.load_library() loaded successfully but both tf.io and tf.python.lib.io.file_io threw unimplemented. Maybe we need a different way to load the plugins ?

Oh, so there is a bug that we'll have to check. I'll be looking on this next week, after I unblock you with the TranslateName change.

Is tf.load_library() the right way to load the plugin into filesystem or there is a specific way to load a plugin ?

tf.load_library() should be the only way to load the plugins into TF.

tf.load_library() should be the only way to load the plugins into TF.

there is a problem with tf.load_library(), it threw unimplemented in macos, windows, linux even after I load the shared object libgcs_filesystem.so/dylib/dll. I have tried to build a shared object for modular_filesystem as well and load it into tensorflow. but after loading successfully 2 shared object, the problem still happened

This is interesting. I'll try to replicate. Most likely we have an ODR violation which I'll have to fix.

@mihaimaruseac
It seems that this RFC is related to our project. I have just skimmed through it but I think if this RFC is accepted, we will have to rewrite our cloud filesystem plugins ?
https://github.com/tensorflow/community/blob/8ae88c1f8ac844a75dc901c017d696e1f7fd7c16/rfcs/20200505-transactional-fs.md

Hi. Yes, they are related though I am thinking that the implementation won't change too much and that there can be collaboration between these two approaches.
That API is still in flux and might change. But if you want to give a review there that should be good.

@mihaimaruseac
I have rewrited the s3 plugins as well. So I will wait for you to review codes for both plugins gcs and s3. In the mean time, I will learn more about hdfs and Transactional File Systems Support

Awesome. I shall look over these two PRs promptly.

because we make use of google-cloud-cpp íntead of write our own client. So we have to rewrite everything

About c++17, if we want to use std::string_view, we have to use it. However, this is plugins-wide only and does not affect anything in Tensorflow.

std::string_view and C++17, I think that should be ok. Otherwise we could try using absl::string_view which works in C++11/14. Since it's easy to change, let's not worry about it for now.

I will have to get some info on how gcs filesystem is used internally and determine if all internal users would be ok with a complete rewrite instead of only an API change with minimal non-API changes. Should get an answer by mid f next week, sorry for the new delay.

Ok. Don't worry about the delay

And could you helo me with something ? While getting information about the API change, would you like to help me get some information about how Tensorflow use CreateDirinternally, especially TensorBoard. As CreateDir is used for creating directory to store file (because local filesystem does not allow store file when parent directory does not exist). I am thinking of a CreateDir which does nothing, just sets the status to Ok.

And there is a gcs smoke test. maybe it could help. if you need the compiled plugins, please tell me. I have gcs plugins on linux, macos and windows too
https://github.com/tensorflow/tensorflow/tree/9590c4c32dd4346ea5c35673336f5912c6072bf2/tensorflow/tools/gcs_test

One issue I noticed is that the implementation tries to download the files locally. There a couple of issues with that approach:

Write throughput on local disk is limited.
We will need to wait for the file to be fully downloaded before we can return the first batch. This introduces a lot of latency to the user.
I'm sure there are other details in the implementation that a rewrite would miss. I suggest updating the previous implementation to use the pluggable interface first, and then updating it step by step to switch to the google cpp client instead to clean up the code.

@sshrdp
I have thought about the issue you said with the first batch download lately. I will check the current implementation as you suggest. Thank you

@sshrdp @mihaimaruseac
I find it is very hard to port the current implementation to Filesystem C API. Because it is heavily dependent on other part of Tensorflow (core/platform/mutex and core/platform/env).
Do you have any idea about this problems ?

Afaik, we should be able to unentangle the current implementation so it only depends on stuff exposed on the stable C API. If there's something missing, we can expand the API with new PRs.

@mihaimaruseac
I have just skimmed throught the current implementation. As I understand, It used a LRU cache filesystem underlying. When the users want something on the server, we will fetch it to a cache file named "filename@offset". Nextime, we will check if the file that the users want exists in cache file or not, if the range is enough or we need to fetch more range, ...
I think I can implement this without use the stuff in Tensorflow Core. However, problems come when we use multithread.
I think I will make a complete rapport about what the current implementation do and how I deal with it tomorrow or the day after. I will upload it to the filechange so we could discuss easily

That sounds great. Thank you

@mihaimaruseac @sshrdp
I have added a design-proposal.md to file changes. Please take a look at it and tell me if I am missing something. Thank you

@mihaimaruseac
By the way, I have finished porting file_block_cache, ram_file_block_cache and expiring_lru_cache to Modular Filesystem. I rename file_block_cache to block_cache in case we want to add file_block_cache ( save cached file to local file rather than to memory ).
In fact, I put those files into a seperate folder plugins/cache since cloud filesystems may need it. Should I make a new PR for those files or just push those files to here so you can take a look at ?

Let's split them into another PR as this is already large.

@mihaimaruseac
Sorry I have misunderstood your review.

Alternatively, we can add one API entry to the filesystem ops set_filesystem_option(name, value) and use it in GCS to control throttling this way, since caller code might know better about what throttling levels are acceptable.

The problem with GCS Throttle isn't how we set the throttling level. It is we have no way to pass it to google-cloud-cpp. The same thing holds for timeout related parameters. I think the best solution is fork google-cloud-cpp and add more control

@sshrdp how important is to have control over GCS throttle?


FixItApplier is not applying all Fix-Its

Description
FixItApplier is not applying all Fix-Its
https://github.com/apple/swift-syntax/pull/1549#discussion_r1179887421
https://github.com/apple/swift-syntax/pull/1549#discussion_r1169231555
Steps to Reproduce
No response

Tracked in Apple’s issue tracker as rdar://108652253

Fixed by https://github.com/apple/swift-syntax/pull/2226