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

remove std_ prefix from math library #431

Merged
merged 8 commits into from
Mar 18, 2021
Merged

remove std_ prefix from math library #431

merged 8 commits into from
Mar 18, 2021

Conversation

rachitnigam
Copy link
Contributor

No description provided.

@rachitnigam
Copy link
Contributor Author

Hm, the current problem with this is the relay backend generates the functions with the same name as the input relay function. This means that the sqrt call in sqrt.relay generates a component called sqrt which conflicts with the definition of sqrt imported from the library. @cgyurgyik any thoughts on how to fix this? We could come up with a naming convection for the methods generated from the relay frontend.

@cgyurgyik
Copy link
Collaborator

Hm, the current problem with this is the relay backend generates the functions with the same name as the input relay function. This means that the sqrt call in sqrt.relay generates a component called sqrt which conflicts with the definition of sqrt imported from the library. @cgyurgyik any thoughts on how to fix this? We could come up with a naming convection for the methods generated from the relay frontend.

Haha yep that's initially why I threw on the std_ prefix. Naming convention for components seems like an easy fix. Perhaps just some type of prefix appended (given its pretty simple), such as r_, which isn't very informative, but is short. I'm open to better suggestions. I wonder how difficult a less error-prone solution would be, such as:

component relay::sqrt(...) -> (...) { ... }

, or

relay {
  component sqrt(...) -> (...) { ... }
}

I imagine this won't be the last time where name collisions occur.

@rachitnigam
Copy link
Contributor Author

Name resolution is hairy business. If we aren't careful with it, it can get complicated. This also ties into @sampsyo's discussion on modules. Maybe we can start a thread there.

@cgyurgyik
Copy link
Collaborator

As suggested, the Relay components now have their arity appended, e.g. relu with output Tensor[1, 10] will be named: relu_1_10.

@rachitnigam
Copy link
Contributor Author

Great, thanks!

@rachitnigam rachitnigam merged commit 88612fb into master Mar 18, 2021
@rachitnigam rachitnigam deleted the rename-math branch March 18, 2021 02:30
sgpthomas pushed a commit that referenced this pull request Mar 22, 2021
* remove std_ prefix from math library

* update test

* update relay impl

* Add prefix and suffix to Relay components.

* Add arity only.

* Remove re.

Co-authored-by: Chris Gyurgyik <37983775+cgyurgyik@users.noreply.github.com>
Co-authored-by: cgyurgyik <gyurgyikcp@gmail.com>
sgpthomas added a commit that referenced this pull request Mar 23, 2021
* add test for minimize regs rewriting conditions

* changing trait bounds + improved doc comments

* add `sharing_component.rs` to abstract common details between minimize-regs and
resource-sharing. fixes #442

* documentation + clippy fixes

* [fud] Errors in Fud prints out STDOUT

* Attempt to serialize workflows (#432)

* Give @external semantics for verilog backend (#430)

* Give @external semantics for verilog backend

@external(1) on a memory makes the backend generate code to initialize
memories and dump out their final states. Every other memory is ignored.
Fud is stricter about what memories it expects in the inputs and
outputs.

* fmt

* relay memories are external

* fmt

* line nonsense

* more reformat

* more test fixing

* Tests

* Commas are the worst

* undo change to pow test

Co-authored-by: cgyurgyik <gyurgyikcp@gmail.com>

* unify mod and div implements (#433)

* remove std_ prefix from math library (#431)

* remove std_ prefix from math library

* update test

* update relay impl

* Add prefix and suffix to Relay components.

* Add arity only.

* Remove re.

Co-authored-by: Chris Gyurgyik <37983775+cgyurgyik@users.noreply.github.com>
Co-authored-by: cgyurgyik <gyurgyikcp@gmail.com>

* Test Language Tutorial examples (#436)

* Test Language Tutorial examples

* update doc link

* add data.json

* link to calyx files

* [docs] Tutorial for memory by reference. (#441)

* Tutorial for memory by reference.

* Update english

* runt

* Use mod output.

* Update Summary.

* rename.

* Fix test.

* [obvious] Change table to columns.

* [obvious] Small grammatical changes.

* [obvious] Fix typo.

* Parser tweaks (#443)

* added check to ensure literal values are representable in the given bitwidth

* added a few tests

* fix edge case

* fix my formatting crimes

* appease clippy

* formatting, again

* fixed condition register test, removed rename swap

Co-authored-by: Rachit Nigam <rachit.nigam12@gmail.com>
Co-authored-by: cgyurgyik <gyurgyikcp@gmail.com>
Co-authored-by: Chris Gyurgyik <37983775+cgyurgyik@users.noreply.github.com>
Co-authored-by: Griffin Berlstein <griffin@berlste.in>
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