Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom_deserialisation argument to BasePass.from_dict #1647

Merged
merged 25 commits into from
Nov 7, 2024

Conversation

sjdilkes
Copy link
Contributor

@sjdilkes sjdilkes commented Nov 1, 2024

Description

Replaces the use of nlohmann::json implicit construction of c++ objects from json using json and from_json with a new method deserialise that directly produces the PassPtr object. This allows a new argument custom_deserialisation to be supplied, which is a map between std::string corrresponding to CustomPass label and a Circuit->Circuit function that can be used to generate a customPass.

Related issues

Please mention any github issues addressed by this PR.

Checklist

  • I have performed a self-review of my code.
  • I have commented hard-to-understand parts of my code.
  • I have made corresponding changes to the public API documentation.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have updated the changelog with any user-facing changes.

[](const py::dict &base_pass_dict,
const std::map<
std::string, std::function<Circuit(const Circuit &)>>
&custom_deserialisation) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This argument should be optional, then we don't break the API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -54,7 +54,7 @@ class BasePass:
:param after_apply: Invoked after a pass is applied. The CompilationUnit and a summary of the pass configuration are passed into the callback.
:return: True if pass modified the circuit, else False
"""
def to_dict(self) -> dict:
def to_dict(self) -> json:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't this still return a dict?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is auto generated based on the serialise method now returning json in the c++

Comment on lines +302 to +308
nlohmann::json serialise(const BasePass& bp);
nlohmann::json serialise(const PassPtr& pp);
nlohmann::json serialise(const std::vector<PassPtr>& pp);

PassPtr deserialise(
const nlohmann::json& j,
const std::map<std::string, std::function<Circuit(const Circuit&)>>&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we called these to_json and from_json as before, can we avoid some of the changes in CompilerPass.cpp (not sure how the magic in nlohmann::json works).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(And also change the signature to match the signatures in JSON_DECL but with the addition of the one optional argument.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My look through the nlohmann docs/code suggested that this wasn't possible, with the nlohmann magic relying one using well defined to_json/from_json with two arguments - the json and the object. So I don't think this is possible

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add new tests for cusom-pass round serialization.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@yao-cqc
Copy link
Contributor

yao-cqc commented Nov 1, 2024

What about typing custom_deserialisation std::map<std::string, std::function<PassPtr(const ConfigType &)>>?
Here ConfigType can be something like nlohmann::json (I don't know what is the best), and it's used for parametrise the pass.

Then we can do something like this:

config ={
            "optimisation_level":optimisation_level,
            "seed":seed
        }
def get_routing_pass(config:Dict) -> CustomPass:
    def transformation(circ: Circuit) -> Circuit:
        # code for transforming the circuit
        # that uses the config
        return Circuit()
    return CustomPass(
        transformation,
        label="MyPass",
        config=config
        )

# serialisation
serialised_pass = get_routing_pass(config).to_dict()
# deserialisation
deserialised_pass = BasePass.from_dict(
    serialised_pass,
    custom_deserialisation={
        "MyPass": get_routing_pass
    }
)   

This does require adding a config field to CustomPass.

@sjdilkes sjdilkes marked this pull request as ready for review November 6, 2024 18:53
@sjdilkes sjdilkes requested a review from cqc-alec November 6, 2024 18:53
@sjdilkes
Copy link
Contributor Author

sjdilkes commented Nov 6, 2024

What about typing custom_deserialisation std::map<std::string, std::function<PassPtr(const ConfigType &)>>? Here ConfigType can be something like nlohmann::json (I don't know what is the best), and it's used for parametrise the pass.

Then we can do something like this:

config ={
            "optimisation_level":optimisation_level,
            "seed":seed
        }
def get_routing_pass(config:Dict) -> CustomPass:
    def transformation(circ: Circuit) -> Circuit:
        # code for transforming the circuit
        # that uses the config
        return Circuit()
    return CustomPass(
        transformation,
        label="MyPass",
        config=config
        )

# serialisation
serialised_pass = get_routing_pass(config).to_dict()
# deserialisation
deserialised_pass = BasePass.from_dict(
    serialised_pass,
    custom_deserialisation={
        "MyPass": get_routing_pass
    }
)   

This does require adding a config field to CustomPass.

I think we would have to force the config to be std::map<std::string, int>to make thetypework at the c++ level. Personally I don't think not being able to pass parameters is a problem, as the user has fine control over what they pass tocustom_deserialisation`

Copy link
Collaborator

@cqc-alec cqc-alec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, just a couple of small comments.

Please also update changelog.

tket/src/Predicates/CompilerPass.cpp Outdated Show resolved Hide resolved
},
"Construct a new Pass instance from a JSON serializable dictionary "
"representation.")
"representation.",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs more documentation of the custom_deserialization parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

@sjdilkes sjdilkes requested a review from cqc-alec November 7, 2024 10:56
@sjdilkes sjdilkes merged commit a9add4b into main Nov 7, 2024
32 checks passed
@sjdilkes sjdilkes deleted the custom-deserialisation-basepass branch November 7, 2024 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants