Use struct for NodeDurations in both ALAP and ASAP Scheduling Passes

[raynelfss]

Summary

The following commits introduce a PyO3 class, NodeDurations, that represents a mapping between DAGOpNode instances and their duration values in either units of dt or seconds.

The main purpose of this structure is to avoid the costly conversion from PyDict to HashMap and back in each instance.

The following PR also restructures the directories for scheduling passes to allow this struct to live in its own file under a common directory.

Details and comments

[raynelfss]

Does this PR need a release note? Although it is a very internal change, the property slot that stores it is mentioned in documentation, but its type is never mentioned explicitly.

[qiskit-bot]

One or more of the following people are relevant to this code:

• @Qiskit/terra-core

[alexanderivrii]

Thanks Ray, I have left a few questions (in particular there is something about the equality of PyNodeDurations that I do not quite understand).

[alexanderivrii]

Ray, thanks for explanations and quick updates. Based on Matthew's comment that node durations should only be used for look-ups by node id, I think that using HashMap to represent node durations and iterating over DAG's Delay nodes in dynamical decoupling both make sense.

This PR does change the type of property_set["node_duration"] from dict to class 'qiskit._accelerate.scheduling.NodeDurations, however I don't think this poses a problem from the backward compatibility standpoint.

As we discussed during the meeting today, we need to preserve backward compatibility as much as possible (in particular we need to go back to using IndexMap).

[raynelfss]

@alexanderivrii I reverted the changes back to using IndexMap and we're lucky to have done so as during testing I found a performance regression I had accidentally introduced 😅. The PR should be in a much better state now.

[alexanderivrii]

Thanks for the updates, Ray!

I did not fully understand where the performance regression that you mentioned was coming from, is it from using HashMap instead of IndexMap?

Overall this looks very nice to me. I believe to fully look a Python dict, you should also implement the __iter__ magic method, what do you think?

Otherwise, it's ready to merge.

[raynelfss]

I did not fully understand where the performance regression that you mentioned was coming from, is it from using HashMap instead of IndexMap?

Oh, I forgot to explain it but it was part of the commit message of the last commit:

cf08009

Fix: Pre-computing error in __getitem__
- An oversight allowed an error variant to precompute itself even if the result was `Ok` via `ok_or`. Replaced it with `ok_or_else`.
...

The regression I introduced simply called Python's __repr__ method on each node regardless of if it was to be used for an error message or not, which caused the performance to worsen greatly.

I believe to fully look a Python dict, you should also implement the iter magic method, what do you think?

I might be wrong but because the struct is specified to be a mapping and it implements the __getitem__, __setitem__ and __len__ methods, Python provides a default implementation. Or at least that what it's supposed to do. I haven't tested this so allow me to confirm that it is possible.

Update: it in fact does not make it iterable...

[raynelfss]

Added iteration methods (__iter__, keys and values) in 95ccf0c

[alexanderivrii]

Thanks for the explanations, and thanks for remembering to also implement keys and values. After experimenting with the code, the NodeDurations data structure now looks like a dict and quacks like a dict, therefore for all practical purposes it's backwards compatible.