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

Signal propagation after topological sorting #9

Merged
merged 3 commits into from
Jun 12, 2014
Merged

Signal propagation after topological sorting #9

merged 3 commits into from
Jun 12, 2014

Conversation

shashi
Copy link
Member

@shashi shashi commented Jun 11, 2014

Tries to address #4 .

# A topological order
begin
local counter = uint(1)
const topo_rank = Dict{Signal, Uint}()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why use a global Dict when you could just store the rank in the Signal?

Copy link
Member Author

Choose a reason for hiding this comment

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

It made sense to keep the topological sorting logic out of Signal's structure. Besides there are many different signal types. This actually takes less code.

Copy link
Contributor

Choose a reason for hiding this comment

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

The code saving is only the saving of adding a rank field to each Signal type. But the result is slower and, I think, more awkward.

(This is similar to the question of callbacks discussed recently with @one-more-minute.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, that actually resulted in 4 lines less code :) c26c97b Thanks! Okay, I'll resist from doing this Dict thing again (pardon the slight mental disorder ;) ).

I guess I am just scared of letting code outside the library being able to so easily edit rank. I wish there was a way to make the signal object immutable except for the value field.

Copy link
Contributor

Choose a reason for hiding this comment

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

You want something like C++'s private members. Julia doesn't enforce this kind of thing at the language level: it just relies on a cultural convention that external code generally shouldn't modify struct members directly (and if it does so, it does so at its own risk).

@shashi
Copy link
Member Author

shashi commented Jun 12, 2014

Do you think push! could be made more efficient in any way? This works and doesn't break the previous API. I think this is good to merge into master.

shashi added a commit that referenced this pull request Jun 12, 2014
1. Use topological sorting for signal proagation
2. More signal types: Filter, SampleOn, Merge, DropRepeats. Node is now abstract
@shashi shashi merged commit 7595ef2 into master Jun 12, 2014
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.

2 participants