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

A clear directory structure to make it simpler to add new processors #217

Open
quantshah opened this issue Sep 25, 2023 · 8 comments
Open

Comments

@quantshah
Copy link
Member

Hi @BoxiLi I am starting to build new devices and noticed that we have a slightly split file/directory structure for the pulse-level simulation code that could be more modular/compact. In any new device, I use the following code structure:

  • custom_processor/
    • model.py
    • processor.py
    • compiler.py
    • scheduler.py
    • noise.py
    • transpiler.py

In many cases, these files will just use the qutip-qip code, e.g., the qutip-qip scheduler or the ZZCrossTalk noise. But nevertheless, having this clear folder structure makes things very easy to develop when adding new simulators. Different people working on different aspects of the processor can work independently.

In the code right now, we have files that are spread over different folders, e.g., device/modelprocessor.py that could be processor/modelprocessor.py. Similarly noise.py is just a file and perhaps coule be noise/coherent.py, noise/crosstalk.py. Similarly we have a scheduler in the compiler/scheduler.py as well as a scheduler folder.

I propose to slowly restructure the folder in the main qutip-qip under model, processor, compiler, scheduler, noise and a transpiler folder structure. E.g.:

What do you think?

- src/qutip_qip/
    - model/
        - model.py 
        - spinchainmodel.py
        - cqedmodel.py
    - processor/
        - modelprocessor.py
        - spinchainprocessor.py
        - cqedprocessor.py
    - compiler/
        - simplecompiler.py
        - spinchaincompiler.py
        - cqedcompiler.py
    - scheduler/
        - alapscheduler.py
    - noise/
        - decoherence.py
        - crosstalk.py
    - transpiler/
        - chain.py 
@hodgestar
Copy link
Contributor

I've also found the current structure a little bit tricky to navigate. Would it be possible to use one file per Hamiltonian model?

Often the processor and model share concepts implicitly (e.g. structure of Hamiltonian and its terms) or explicitly (e.g. relying on the presence of methods or variables), so it would be nice to keep them together.

It would also be nice to make it clearer what the APIs for the Model and Processor are, by having simple base classes that don't implement the methods themselves and make it clear what implementations should overwrite.

It would also be good for the functionality to be aggregated rather than inherited. E.g. If there is shared functionality, put it in a separate module, function or class, rather than having inheritance of complex behaviour.

These models are a super useful and important part of qutip-qip.

@quantshah
Copy link
Member Author

quantshah commented Sep 26, 2023

@hodgestar I agree that the model and processor belong together. But still it is very nice to be able to clearly just read the model out, get the Hamiltonians etc and therefore I am more inclined to keep it separate as a model.py file. I agree that we could have a better structure and perhaps make one folder per Hamiltonian/device, e.g.,

spinchain/
        model.py
        processor.py
        compiler.py
        scheduler.py
        noise.py
        transpiler.py

The part about making the API clearer is also valid. E.g., I want to know that in the Model, I only need to define how to set the _drift and _control terms along with params. I will try to add these changes in separate PRs but will wait for @BoxiLi 's comments.

@hodgestar
Copy link
Contributor

Having the model and processor classes in one file doesn't prevent them being used independently.

@BoxiLi
Copy link
Member

BoxiLi commented Sep 27, 2023

Thank you both!

Would it be possible to use one file per Hamiltonian model?

I agree that a structure like this makes it easier to import different modules for the same processor.

spinchain/
        model.py
        processor.py
        compiler.py
        scheduler.py
        noise.py
        transpiler.py

The problem I experienced with this structure is that if two processors share the same model/noise/transpiler, one has to create a Base processor for it. If spinchain1 and spinchain2 share the same compiler, but spinchain1 and circuitqed share the same transpiler, it becomes a bit cumbersome to implement this.

I propose to slowly restructure the folder in the main qutip-qip under model, processor, compiler, scheduler, noise and a transpiler folder structure.

Yes, I agree that creating a folder for each of them makes the structure clearer. There is a lot that can be improved on the naming.

About model and processor, actually, a while ago I tried to decouple them. Keeps only ModelProcessor, which takes a Hamiltonian model and predefined compilation routines, and OptPulseProcessor, which uses optimal control algorithms to generate the pulse. In this way, a processor for a specific hardware can be built by providing with it the related model, compiler, transpiler etc. There will be no class SpinChain with everything about spinchain included, instead one needs

spinchainprocessor = ModelProcessor(..., model=model, transpiler=transpiler, compiler=compiler)

I'm wondering which one is better, the one above or the current one

spinchainprocessor = SpinChain(..., params=params)

with model and everything implicitly included in the class `SpinChain?

@hodgestar
Copy link
Contributor

I quite like the more split out one:

spinchainprocessor = ModelProcessor(..., model=model, transpiler=transpiler, compiler=compiler)

we can always introduce simple functions or classes to make it easier for new users to produce them, e.g.

class SpinChainProcessor(ModelProcessor):
    def __init__(self, ...):
        model = ...
        transpiler = ...
        compiler = ...
        super().__init__(..., model=model, transpiler=transpiler, compiler=compier)

@quantshah
Copy link
Member Author

I also quite like the following design @hodgestar

compiler = SpinChainCompiler(num_qubits, gate_params ...)
spinchainprocessor = ModelProcessor(..., model=model, compiler=compiler, transpiler=transpiler, scheduler=scheduler)

But I faced an issue where the compiler must know the model. The compiler initialization could include the gate times and parameters needed to produce the pulses by the compiler. Still, the compiler also requires information about the model, native gates etc. So, should it get the processor as an input?

I don't know what will be the best design for the individual parts. E.g., should the compiler assume that there is a model.params available? What about running the compiler independently outside the processor using compiler.compile(circuit)? Maybe we can make sure that none of the methods for the compiler (and others) can be run outside the processor. Then, we could be reasonably sure that when we call a compiler.compile(circuit), we are assured that a self[processor].model.controls exists, as well as a self[processor].compiler.gate_time exists.

@BoxiLi I am developing a new processor and can work on this over the next few weeks. We could work out these details to also update the qutip-qip module as a whole. Do you have some time to discuss these things in detail?

@quantshah
Copy link
Member Author

I was also thinking that we should make the Processor, Model, Compiler .... abstract classes to enforce that certain methods have to be defined, e.g., model.get_control_hamiltonian(), model.load_circuit() ...

I am reading up on abstract classes in Python here:
https://earthly.dev/blog/abstract-base-classes-python/

@BoxiLi
Copy link
Member

BoxiLi commented Sep 28, 2023

But I faced an issue where the compiler must know the model.

Yes... That is true, to initialize the compiler it needs the hardware parameters. Getting processor as an input is too much, it makes the dependency a bit circular. I should be a dictionary of parameters (saved in Model). Maybe, these parameters only need to be given at runtime? i.e. as an input for compiler.compile.

What I usually do is to first define Model and then pass Model.params to Compiler.

I am developing a new processor and can work on this over the next few weeks. We could work out these details to also update the qutip-qip module as a whole. Do you have some time to discuss these things in detail?

That is great! We could do e.g. next week?
It would be great if we could improve the package while developing a new processor.

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

No branches or pull requests

3 participants