microsoft / qsharp

Azure Quantum Development Kit, including the Q# programming language, resource estimator, and Quantum Katas
https://microsoft.github.io/qsharp/
MIT License
367 stars 73 forks source link

Simplify dependencies handling for exercises in katas #1640

Closed tcNickolas closed 14 hours ago

tcNickolas commented 2 weeks ago

Is your feature request related to a problem? Please describe. Currently each exercise in each kata has a field qsDependencies that lists files that need to be compiled together with the files of that specific exercise.

@[exercise]({
    "id": "marking_oracles__kth_bit",
    "title": "K-th Bit",
    "path": "./kth_bit/",
    "qsDependencies": [
        "../KatasLibrary.qs",
        "./Common.qs"
    ]
})

This is very repetitive, leads to hard-to-debug issues when contributing to the katas, and unnecessary, since the logic of including the files is the same across all katas.

Describe the solution you'd like I'd like to drop this field and instead, when an exercise is created, to include the files using the following logic:

  1. Always include KatasLibrary.qs.
  2. If the kata to which this exercise belongs has a Common.qs file, include it.

Describe alternatives you've considered We can keep the current approach.

ggridin commented 5 days ago

@tcNickolas I have few clarifying questions:

  1. Does it make sense to assume that KatasLibrary.qs is always 1 level above index.md (as of now)? If not, the logic could be adjusted to "If the kata to which this exercise belongs has ../KatasLibrary.qs (1 level up) file, include it".
  2. After this change, qsDependencies will be optional property, right?

May I work on this issue?

tcNickolas commented 4 days ago
  1. Yes, looking at the current infra, including katas placed elsewhere into it would take significant changes, and we're not planning to do that. It's probably a good idea not to fail if the file doesn't exist, but we don't need to account for a possibility of it existing two levels up or in a completely different folder.
  2. We shouldn't need qsDependencies property at all! It was designed with just the first small set of katas as pilots, and now that we've added a lot more katas, we can see that we don't need to over-complicate their definitions, since they all follow the same structure easily.

Yes, absolutely!

ggridin commented 4 days ago

Clarification for question 2 Am I correct that I should remove processing of qsDependencies property completely and remove all it's references from index.md's?

tcNickolas commented 4 days ago

Yes please!

ggridin commented 3 days ago

@tcNickolas I have the fix. What upstream branch should I use to track the changes? I do not have access to create remote branch.

tcNickolas commented 3 days ago

You'll need to fork the microsoft/qsharp repository to your account, and then you will be able to create a branch in ggridin/qsharp repository and send the pull request from that branch to the main repo.