Add in qpy the support for Discriminator and Kernel #10327

junnaka51 · 2023-06-23T09:49:48Z

Summary

This PR intends to solve #10050 by adding in qpy the support for Discriminator and Kernel.
This enables users to serialize Acquire instructions with Discriminator and Kernel using qpy.

Details and comments

The following are from #10050 and show that the error is resolved:

In [1]: from io import BytesIO
   ...: 
   ...: import numpy as np
   ...: 
   ...: from qiskit import QuantumCircuit, pulse
   ...: from qiskit.qpy import dump, load
   ...: 
   ...: 
   ...: qc = QuantumCircuit(1, 1)
   ...: qc.measure(0, 0)
   ...: 
   ...: discriminator = pulse.Discriminator("hw_centroid")
   ...: 
   ...: with pulse.builder.build() as sched:
   ...:     pulse.builder.acquire(10, 0, pulse.MemorySlot(0), discriminator=discriminator)
   ...: 
   ...: qc.add_calibration("measure", (0,), sched)
   ...: 
   ...: file_ = BytesIO()
   ...: dump(qc, file_)
   ...: file_.seek(0)
   ...: new_qc = load(file_)[0]
   ...: print(qc == new_qc)
True

In [2]:

In [1]: from io import BytesIO
   ...: 
   ...: import numpy as np
   ...: 
   ...: from qiskit import QuantumCircuit, pulse
   ...: from qiskit.qpy import dump, load
   ...: 
   ...: 
   ...: qc = QuantumCircuit(1, 1)
   ...: qc.measure(0, 0)
   ...: 
   ...: kernel = pulse.Kernel("hw_qmfk", kernel={"real": np.zeros(10), "imag": np.zeros(10)}, bias=[0, 0])
   ...: 
   ...: with pulse.builder.build() as sched:
   ...:     pulse.builder.acquire(10, 0, pulse.MemorySlot(0), kernel=kernel)
   ...: 
   ...: qc.add_calibration("measure", (0,), sched)
   ...: 
   ...: file_ = BytesIO()
   ...: dump(qc, file_)
   ...: file_.seek(0)
   ...: new_qc = load(file_)[0]
   ...: print(qc == new_qc)
True

In [2]:

qiskit-bot · 2023-06-23T09:49:52Z

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

@Qiskit/terra-core
@mtreinish
@nkanazawa1989

coveralls · 2023-06-23T13:36:19Z

Pull Request Test Coverage Report for Build 5606590344

73 of 81 (90.12%) changed or added relevant lines in 3 files are covered.
4 unchanged lines in 1 file lost coverage.
Overall coverage increased (+0.02%) to 86.06%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/pulse/configuration.py	20	24	83.33%
qiskit/qpy/type_keys.py	3	7	42.86%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	4	90.89%

Totals
Change from base Build 5606539535:	0.02%
Covered Lines:	72897
Relevant Lines:	84705

💛 - Coveralls

wshanks

This generally looks good to me. The tests demonstrate the behavior we want. Based on my comments on the nested dict equality function, we might want some tests of Kernel/Discriminator equality.

A qpy expert should review the qpy changes (@jakelishman @mtreinish @nkanazawa1989). A qpy the LIST handling concerns me a bit -- it only works for json-serializable lists. Also, qpy tries to be compact, so falling back to json feels like it is out of character. I wonder if the sequence type could be used? One issue is that the kernel/discriminator parameters are open-ended in schema (I guess whatever the qobj json serializer supports).

qiskit/qpy/type_keys.py

qiskit/pulse/configuration.py

wshanks · 2023-06-23T18:41:25Z

qiskit/pulse/configuration.py

+            return _assert_nested_dict_equal(self.__dict__, other.__dict__)
+        return False
+
+    def __hash__(self):


Was there a need for the hash? I didn't notice the kernel being used in a set or dictionary.

If Kernel is unhashable, the following test fails:
https://github.com/Qiskit/qiskit-terra/blob/b73115f80bc575b5dbc8b98ad003cd3b4ff9c00d/test/python/pulse/test_instructions.py#L68-L101

Hmm, I am not sure what the best approach is here.

The current behavior is that the kernel and discriminator use Python's default hashing and equality behavior where every instance gets a unique hash and the only equality is that an instance is equal to itself. So they are hashable but not in a particularly useful way. Overriding the default __eq__ method unsets the default __hash__ method. I wonder if other behavior relies on pulse instructions being hashable? I have not tried to check.

The kernel and discriminator classes are not well-specified -- basically just json serializable structures (extended to support numpy arrays) -- so nested dicts and lists of float, int, str, and ndarrays. That worked fine for qobj but does not fit as well for qpy. They don't need to be mutable so hashing the data should be okay. I think a more proper hash would be to replace lists with tuples and dicts with tuples of key-value tuples recursively, but that is a bit involved, especially if we don't really need hashing.

I wonder if other behavior relies on pulse instructions being hashable? I have not tried to check.

SymbolicPulse is not hashable:
https://github.com/Qiskit/qiskit-terra/blob/e9f8b7c50968501e019d0cb426676ac606eb5a10/qiskit/pulse/library/symbolic_pulses.py#L570

So, not all of instructions are actually hashable.

I think a more proper hash would be to replace lists with tuples and dicts with tuples of key-value tuples recursively, but that is a bit involved, especially if we don't really need hashing.

Thank you so much for your suggestion!
Although I am still not sure whether Kernel and Discriminator should remain hashable (because the other operand SymbolicPulse is unhashable and as a result not all of instructions are hashable), as long as we cannot eliminate the possibility that their hashing is used somewhere, I suppose that it is safer to keep them hashable.
I updated the hash methods following your suggestion :) 👉 0afd18d

I agree hash is no longer needed. The Kernel and Discriminator haven't been really maintained since they were implemented (last update was Apr. 2020!). At the time pulse module didn't take any Parameter object and all instructions were immutable. I think this is why they implement the hash function. Feel free to drop hash, because they are no longer useful.

TsafrirA

This is still WIP, so I am guessing the documentation and release notes will follow later, but I do want to raise the question if QPY version should be bumped.

Additionally, the compatibility test should be updated.

TsafrirA · 2023-06-25T08:17:12Z

qiskit/qpy/common.py

@@ -18,7 +18,7 @@
 import io
 import struct

-from qiskit.qpy import formats
+from qiskit.qpy import formats, type_keys

 QPY_VERSION = 7


Should QPY version be bumped?

I was not sure. I would like to hear from a QPY expert.

Version update is not necessary, because this change doesn't break any existing QPY data. However, if you want to be really strict, you can still bump the version so that you can raise a user warning when an acquire instruction is loaded from an old QPY data. Recall that discriminator and kernel are silently dropped before this change; if a user (maybe a client) dumps his/her circuit with QPY v7 and sends it to another user (maybe a host server) using v7+, the kernel and discriminator are accidentally dropped through this communication.

This guidance was a little surprising to me at first since we are adding a new data type, but I think it makes sense. Here is how I think about it that makes sense for me:

qpy attempts to provide forwards compatibility for loading previously stored circuits (I think there is still freedom for terra to completely remove something and then it is okay not to load that any more). Since the way data is stored for a specific object might need to change, qpy versions its files and can use the version number to know to load an object stored with the old format. Here we are adding new types to be stored, so the change has no impact on loading previously stored data. The part that was confusing to me is that we could store a kernel in terra 0.25 with qpy version 7 and terra 0.24 would also have qpy version 7 but not be able to load the kernel. However, the terra version is also stored in the qpy data and a separate warning is generated when trying to load qpy data with an earlier version of terra than the one used to generate the data, so I think that covers that case.

TsafrirA · 2023-06-25T08:20:03Z

qiskit/qpy/binary_io/schedules.py

@@ -60,6 +61,22 @@ def _read_waveform(file_obj, version):
    )


+def _read_kernel_and_discriminator(file_obj, version, kernel_or_discriminator):


Unless I am missing a technical reason to have these two objects handled in the same function, I think I would have gone for two separate functions.

I could not find any difference between Kernel and Discriminator at their implementations, so I added the common function for them. But I guess having two separate functions may have more benefits such as ease of maintenance, so I do so :) 👉 e19e938

Co-authored-by: Will Shanks <wshaos@posteo.net>

…ator

junnaka51 · 2023-06-29T14:43:59Z

@wshanks

Based on my comments on the nested dict equality function, we might want some tests of Kernel/Discriminator equality.

Thank you so much for your comments! I added those tests :) 👉 5cf5d3a

junnaka51 · 2023-06-29T14:52:59Z

@TsafrirA

Additionally, the compatibility test should be updated.

Thank you very much for pointing it out! I did not know it.
I added a test here 👉 36f628c
But I am not familiar with this kind of compatibility tests, so any suggestion for improving this would be very much appreciated 🙇

wshanks

This is looking pretty good to me.

I do not like the hashing based on string representation and there are some questions about the qpy details, but I think this is looking good otherwise.

wshanks · 2023-06-29T20:48:39Z

qiskit/qpy/type_keys.py

+        if isinstance(obj, list):
+            return cls.LIST
+        if isinstance(obj, dict):
+            return cls.DICT


I have not dug deeply into the internals of qpy. Why do we need these LIST and DICT types? Why can we not just use SEQUENCE and MAPPING? (I see that MAPPING did not support nesting and you extended it with DICT but could it not be extended with DICT?).

EDIT: at the end what I meant was "could it be extended using nested MAPPING rather than adding DICT?"

I agree. @junnaka51 is there any reason not to use existing types?

I cannot find SEQUENCE and MAPPING as types in https://github.com/Qiskit/qiskit-terra/blob/main/qiskit/qpy/type_keys.py .

If I understand correctly, we need an identifier of LIST (or we can call it SEQUENCE) and that of DICT (or Mapping) for reading them out from a binary.
I now realize that we can use the existing functions such as
https://github.com/Qiskit/qiskit-terra/blob/e9f8b7c50968501e019d0cb426676ac606eb5a10/qiskit/qpy/common.py#L203
instead of upgrading qpy.binary_io.value.dumps_value() and qpy.binary_io.value.loads_value(). But I think we still need the identifiers. For example, when we want to deal with LIST as a value in a DICT, then we may update qpy.common.write_mapping() as

def write_mapping(file_obj, mapping, serializer, **kwargs): """Write a mapping in the file like object. """ num_elements = len(mapping) file_obj.write(struct.pack(formats.SEQUENCE_PACK, num_elements)) for key, datum in mapping.items(): key_bytes = key.encode(ENCODE) + if isinstance(datum, list): + type_key, datum_bytes = b"l", sequence_to_binary(datum, serializer, **kwargs) + else: + type_key, datum_bytes = serializer(datum, **kwargs) - type_key, datum_bytes = serializer(datum, **kwargs) item_header = struct.pack(formats.MAP_ITEM_PACK, len(key_bytes), type_key, len(datum_bytes)) file_obj.write(item_header) file_obj.write(key_bytes) file_obj.write(datum_bytes)

and qpy.common.read_mapping() as

def read_mapping(file_obj, deserializer, **kwargs): """Read a mapping from the file like object. """ mapping = {} data = formats.SEQUENCE._make( struct.unpack(formats.SEQUENCE_PACK, file_obj.read(formats.SEQUENCE_SIZE)) ) for _ in range(data.num_elements): map_header = formats.MAP_ITEM._make( struct.unpack(formats.MAP_ITEM_PACK, file_obj.read(formats.MAP_ITEM_SIZE)) ) key = file_obj.read(map_header.key_size).decode(ENCODE) + if map_header.type == b"l": + datum = common.read_sequence(file_obj, deserializer, **kwargs) + else: + datum = deserializer(map_header.type, file_obj.read(map_header.size), **kwargs) - datum = deserializer(map_header.type, file_obj.read(map_header.size), **kwargs) mapping[key] = datum return mapping

, in both of which b"l" is used as the identifier of LIST (or SEQUENCE if we want).
Of course this b"l" ~~must~~ needs (edited) not be defined in qpy/type_keys.py. But I thought qpy/type_keys.py summarizes all the types, so I put it there.
Any suggestion would be very much appreciated.

You're absolutely right. I think nested dictionary is not a Value type, and indeed I didn't assume this data structure when I wrote this function. Instead of calling value.dumps_value, you can write a custom serializer function for discriminator/kernel config, which would look like

def my_custom_serializer(obj, version): if isinstance(obj, dictionary): with io.BytesIO() as container: for key, value in obj.items() # recursively generate binary data binary_data = container.getvalue() return b"D", binary_data return dumps_value(obj, version)

what do you think?

@nkanazawa1989 Thank you so much for your suggestion and advice 🙇
Please give me a little more time. It has been taking time to understand it.

Thanks, @junnaka51! By the way, this PR is currently labeled with the 0.25.0 milestone which should be released in about 10 days. There is no pressure to meet the milestone though. We can bump it back to the next milestone if needed. Leaving the milestone on the PR for now means that we can still merge it if it is ready (a newly opened PR would not generally by eligible for 0.25.0 at this point).

@wshanks Thank you for the info! :)

In any case, I think with your last push this PR might be good now? The one thing I wonder about is the lack of type_keys for b"D" and b"l" -- it feels like names would be clearer than plain strings, but I defer to @nkanazawa1989

No, not yet. Although it works, I suppose how to deal with a nested dictionary is not yet well written.
I am trying to grasp @nkanazawa1989 `s comments.

@nkanazawa1989 Thank you again for your advice and suggestion 🙇
I agree that your approach is more sophisticated. I added a new serializer and deserializer comparable in abstraction to value.dumps_value and value.loads_value, respectively, and put write_mapping and read_mapping back to their original 👉 cc878e6

nkanazawa1989

Thanks @junnaka51 for the continuous contribution. The PR looks great. Let me add some comments.

nkanazawa1989 · 2023-06-30T08:44:22Z

qiskit/pulse/configuration.py

+            return _assert_nested_dict_equal(self.__dict__, other.__dict__)
+        return False
+
+    def __hash__(self):


I agree hash is no longer needed. The Kernel and Discriminator haven't been really maintained since they were implemented (last update was Apr. 2020!). At the time pulse module didn't take any Parameter object and all instructions were immutable. I think this is why they implement the hash function. Feel free to drop hash, because they are no longer useful.

nkanazawa1989 · 2023-06-30T08:46:48Z

qiskit/pulse/configuration.py


 from .channels import PulseChannel, DriveChannel, MeasureChannel
 from .exceptions import PulseError


+def _assert_nested_dict_equal(a, b):


Alternatively we can dump configuration data into string with IQX JSON encoder and compare the raw payload data. But maybe this is unnecessary coupling to IBM provider? @wshanks

How would you do that without making the provider a (circular) dependency of terra?

I think this function is okay. Maybe a comment could be added that the only reason it is needed is because ndarray overrides __eq__ in an unhelpful way? The other things the JSON encoder handles are complex and ParameterExpression which work okay with __eq__.

Yes, basically I agree with you. Unfortunately likely Qiskit doesn't have control of Kernel/Discriminator format, so we should continuously maintain this function to sync with the provider. I was thinking it was bit easier to delegate everything to the provider, but from the dependency viewpoint this approach is bit tough.

I think at some point we should define subclass and use something like pydantic rather than this free-form dictionary. Of course not in this PR :)

nkanazawa1989 · 2023-06-30T08:48:31Z

qiskit/qpy/binary_io/schedules.py

+
+
+def _write_discriminator(file_obj, data):
+    _write_kernel(file_obj, data)


I think it's safe to implement the duplicated code because in future we may update other format.

nkanazawa1989 · 2023-06-30T08:57:36Z

qiskit/qpy/common.py

@@ -18,7 +18,7 @@
 import io
 import struct

-from qiskit.qpy import formats
+from qiskit.qpy import formats, type_keys

 QPY_VERSION = 7


Version update is not necessary, because this change doesn't break any existing QPY data. However, if you want to be really strict, you can still bump the version so that you can raise a user warning when an acquire instruction is loaded from an old QPY data. Recall that discriminator and kernel are silently dropped before this change; if a user (maybe a client) dumps his/her circuit with QPY v7 and sends it to another user (maybe a host server) using v7+, the kernel and discriminator are accidentally dropped through this communication.

nkanazawa1989 · 2023-06-30T09:05:19Z

qiskit/qpy/common.py

@@ -165,6 +171,15 @@ def write_mapping(file_obj, mapping, serializer, **kwargs):
    file_obj.write(struct.pack(formats.SEQUENCE_PACK, num_elements))
    for key, datum in mapping.items():
        key_bytes = key.encode(ENCODE)
+
+        if isinstance(datum, dict):


Instead of doing this, you can generate datum_bytes with this trick by recursively calling write_mapping with container in the code below.
https://github.com/Qiskit/qiskit-terra/blob/e9f8b7c50968501e019d0cb426676ac606eb5a10/qiskit/qpy/common.py#L196-L198

nkanazawa1989 · 2023-06-30T09:07:01Z

qiskit/qpy/type_keys.py

+        if isinstance(obj, list):
+            return cls.LIST
+        if isinstance(obj, dict):
+            return cls.DICT


I agree. @junnaka51 is there any reason not to use existing types?

nkanazawa1989 · 2023-06-30T09:07:46Z

test/python/pulse/test_experiment_configurations.py

@@ -93,5 +94,153 @@ def test_get_channel_lo(self):
            lo_config.channel_lo(MeasureChannel(1))


+class TestKernel(QiskitTestCase):


nkanazawa1989 · 2023-06-30T09:10:15Z

test/python/pulse/test_experiment_configurations.py

+        self.assertFalse(hash(kernel_a) == hash(kernel_c))
+
+
+class TestDiscriminator(QiskitTestCase):


wshanks · 2023-06-30T16:22:20Z

qiskit/pulse/configuration.py

+    return True
+
+
+def _get_hash(obj: dict):


This looks good to me, but following @nkanazawa1989's comment I think just removing hashing might be better since we can't think of a good use case and less code to maintain is always better.

wshanks

This all looks totally functional and well structured to me. I don't quite follow why the nested sequence and mapping serialization/deserialization needs to be kept out of the main loads_value/dumps_value functions, but it matches what Naoki suggested.

@nkanazawa1989 Do you want to give final approval and get this merged in time for the next release?

mtreinish

From the QPY side this LGTM, we might want some docs on how KERNEL and DISCRIMINATOR are serialized. But it's not critical and we can do it post merge. The one thing to point out is that this will likely merge conflict with #10392 but the rebase shouldn't be too bad.

junnaka51 · 2023-07-20T08:13:04Z

@wshanks Thank you so much for resolving the conflict! 🙇

junnaka51 · 2023-07-20T23:54:51Z

@wshanks @nkanazawa1989 @TsafrirA @mtreinish Thank you so much for review! 🙇‍♂️

* add support for Discriminator and Kernel * make Kernel and Discriminator hashable * correct _assert_nested_dict_equal() Co-authored-by: Will Shanks <wshaos@posteo.net> * ensure b does not have extra keys Co-authored-by: Will Shanks <wshaos@posteo.net> * devide _read_kernel_and_discriminator and _write_kernel_and_discriminator * update docs * add tests testing Kernel/Discriminator equality * add a qpy compatibility test * add releasenote * improve hashing in Kernel/Discriminator * rm hashing from Kernel/Discriminator * use write/read_sequence() to serialize lists * add (de)serializer for dict and list --------- Co-authored-by: Will Shanks <wshaos@posteo.net> Co-authored-by: Will Shanks <willshanks@us.ibm.com>

add support for Discriminator and Kernel

518dad0

junnaka51 requested review from a team, eggerdj and wshanks as code owners June 23, 2023 09:49

qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Jun 23, 2023

make Kernel and Discriminator hashable

0c0d0a5

junnaka51 mentioned this pull request Jun 23, 2023

qpy does not support Discriminator and Kernel #10050

Open

wshanks reviewed Jun 23, 2023

View reviewed changes

TsafrirA reviewed Jun 25, 2023

View reviewed changes

junnaka51 and others added 7 commits June 28, 2023 22:50

correct _assert_nested_dict_equal()

1b3266a

Co-authored-by: Will Shanks <wshaos@posteo.net>

ensure b does not have extra keys

8a7b468

Co-authored-by: Will Shanks <wshaos@posteo.net>

devide _read_kernel_and_discriminator and _write_kernel_and_discrimin…

e19e938

…ator

update docs

ed7515d

add tests testing Kernel/Discriminator equality

5cf5d3a

add a qpy compatibility test

36f628c

add releasenote

d44dd11

junnaka51 changed the title ~~[WIP] Add in qpy the support for Discriminator and Kernel~~ Add in qpy the support for Discriminator and Kernel Jun 29, 2023

wshanks reviewed Jun 29, 2023

View reviewed changes

jakelishman added this to the 0.25.0 milestone Jun 29, 2023

jakelishman added Changelog: New Feature Include in the "Added" section of the changelog mod: qpy Related to QPY serialization labels Jun 29, 2023

improve hashing in Kernel/Discriminator

0afd18d

nkanazawa1989 reviewed Jun 30, 2023

View reviewed changes

wshanks reviewed Jun 30, 2023

View reviewed changes

mtreinish assigned nkanazawa1989 Jul 10, 2023

rm hashing from Kernel/Discriminator

bbbbccb

junnaka51 added 2 commits July 11, 2023 16:39

Merge branch 'main' into issue-10050

8c42068

use write/read_sequence() to serialize lists

f9a12c7

junnaka51 changed the title ~~Add in qpy the support for Discriminator and Kernel~~ [WIP] Add in qpy the support for Discriminator and Kernel Jul 11, 2023

junnaka51 and others added 3 commits July 13, 2023 13:22

add (de)serializer for dict and list

cc878e6

Merge branch 'main' into issue-10050

cfc026a

Merge branch 'main' into issue-10050

26c6378

junnaka51 changed the title ~~[WIP] Add in qpy the support for Discriminator and Kernel~~ Add in qpy the support for Discriminator and Kernel Jul 13, 2023

junnaka51 requested review from nkanazawa1989 and wshanks July 13, 2023 16:10

Merge branch 'main' into issue-10050

54f1268

wshanks previously approved these changes Jul 19, 2023

View reviewed changes

mtreinish previously approved these changes Jul 19, 2023

View reviewed changes

mtreinish added this pull request to the merge queue Jul 19, 2023

github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Jul 20, 2023

Merge branch 'main' into issue-10050

18adeac

wshanks dismissed stale reviews from mtreinish and themself via 18adeac July 20, 2023 03:32

wshanks enabled auto-merge July 20, 2023 03:33

mtreinish approved these changes Jul 20, 2023

View reviewed changes

wshanks added this pull request to the merge queue Jul 20, 2023

Merged via the queue into Qiskit:main with commit e595066 Jul 20, 2023

kt474 mentioned this pull request Jul 28, 2023

QPY Terra 0.25 updates Qiskit/qiskit-ibm-provider#684

Merged

		@@ -60,6 +61,22 @@ def _read_waveform(file_obj, version):
		)


		def _read_kernel_and_discriminator(file_obj, version, kernel_or_discriminator):



		def _write_discriminator(file_obj, data):
		_write_kernel(file_obj, data)

		@@ -93,5 +94,153 @@ def test_get_channel_lo(self):
		lo_config.channel_lo(MeasureChannel(1))


		class TestKernel(QiskitTestCase):

		self.assertFalse(hash(kernel_a) == hash(kernel_c))


		class TestDiscriminator(QiskitTestCase):

Add in qpy the support for Discriminator and Kernel #10327

Add in qpy the support for Discriminator and Kernel #10327

Conversation

junnaka51 commented Jun 23, 2023 • edited Loading

Summary

Details and comments

qiskit-bot commented Jun 23, 2023

coveralls commented Jun 23, 2023 • edited Loading

Pull Request Test Coverage Report for Build 5606590344

💛 - Coveralls

wshanks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TsafrirA left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junnaka51 Jun 29, 2023 • edited Loading

Choose a reason for hiding this comment

junnaka51 commented Jun 29, 2023

junnaka51 commented Jun 29, 2023

wshanks left a comment

Choose a reason for hiding this comment

wshanks Jun 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junnaka51 Jul 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junnaka51 Jul 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nkanazawa1989 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wshanks left a comment

Choose a reason for hiding this comment

mtreinish left a comment

Choose a reason for hiding this comment

junnaka51 commented Jul 20, 2023

junnaka51 commented Jul 20, 2023

junnaka51 commented Jun 23, 2023 •

edited

Loading

coveralls commented Jun 23, 2023 •

edited

Loading

junnaka51 Jun 29, 2023 •

edited

Loading

wshanks Jun 29, 2023 •

edited

Loading

junnaka51 Jul 2, 2023 •

edited

Loading

junnaka51 Jul 11, 2023 •

edited

Loading