Skip to content
This repository was archived by the owner on May 30, 2025. It is now read-only.

Tuples! - #94

Merged
gussmith23 merged 39 commits into
mainfrom
tuple
Feb 9, 2021
Merged

Tuples!#94
gussmith23 merged 39 commits into
mainfrom
tuple

Conversation

@hypercubestart

@hypercubestart hypercubestart commented Dec 4, 2020 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Included in this PR:

  • new glenside language constructs
    • ConstructTuple
    • TupleGetItem
  • additional Relay Opaque Operators
    • RelayLeakyReLU
    • RelayAvgPool2D
    • RelayUpSampling
    • RelaySigmoid
    • RelayMaximum
    • RelayMinimum
  • edit fn create_worklist in from_relay/mod.rs to efficiently parse larger programs
  • edit run_relay.py to allow tuple output

@hypercubestart hypercubestart mentioned this pull request Dec 8, 2020
Closed
@gussmith23
gussmith23 self-requested a review January 11, 2021 18:10
gussmith23
gussmith23 suggested changes Jan 11, 2021
View reviewed changes

@gussmith23 gussmith23 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

First pass!

Comment thread src/codegen.rs
Comment thread src/codegen.rs Outdated
Comment thread src/codegen.rs
Comment thread src/codegen.rs Outdated
Comment thread Cargo.toml
Comment thread src/language/from_relay/mod.rs
Comment thread src/language/from_relay/mod.rs Outdated
Comment thread src/language/language.rs Outdated
Comment thread src/language/language.rs Outdated
Comment thread src/language/language.rs
gussmith23
gussmith23 reviewed Jan 20, 2021
View reviewed changes

@gussmith23 gussmith23 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

More comments. Also waiting on tests in from_relay, when you get the chance!

Comment thread src/language/from_relay/mod.rs Outdated
Comment thread src/language/language.rs Outdated
Comment thread src/language/from_relay/mod.rs Outdated
Comment thread src/language/language.rs Outdated
@hypercubestart hypercubestart mentioned this pull request Jan 20, 2021
Open
@gussmith23 gussmith23 changed the title [WIP] tuples! Tuples! Jan 25, 2021
gussmith23
gussmith23 reviewed Jan 25, 2021
View reviewed changes

@gussmith23 gussmith23 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Some more comments! Nearly there. These comments are mostly about tests.

Comment thread Cargo.toml
Comment thread src/codegen.rs Outdated
Comment thread src/codegen.rs
Comment thread src/language/from_relay/mod.rs Outdated
Comment thread src/language/from_relay/mod.rs Outdated
Comment thread src/language/from_relay/run_relay.py
Comment thread src/language/from_relay/run_relay.py
Comment thread src/language/language.rs Outdated
Comment thread src/language/language.rs Outdated
Comment thread src/language/language.rs Outdated
gussmith23
gussmith23 approved these changes Feb 9, 2021
View reviewed changes

@gussmith23 gussmith23 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'll merge this once I make sure the tests pass! Thanks so much for all of your hard work!!

@gussmith23

gussmith23 commented Feb 9, 2021

Copy link
Copy Markdown
Owner

@hypercubestart hmm...any idea where this new error would be coming from? https://github.com/gussmith23/glenside/pull/94/checks?check_run_id=1866343438

It seems pretty dumb...it seems like somehow this line is running, even though the path already exists. It's really odd to me that only a single test is failing with this error, too.

@hypercubestart

hypercubestart commented Feb 9, 2021 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

@gussmith23 ah I think i ran into that before, and rerunning the tests worked

@gussmith23

gussmith23 commented Feb 9, 2021

Copy link
Copy Markdown
Owner

🤞 hope so! i already reran them once, didn't work D:

@gussmith23
gussmith23 merged commit fae7d84 into main Feb 9, 2021
@gussmith23
gussmith23 deleted the tuple branch February 9, 2021 22:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@gussmith23 gussmith23 gussmith23 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hypercubestart @gussmith23