Skip to content
This repository has been archived by the owner on Jul 23, 2019. It is now read-only.

Replacement manager #11

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Replacement manager #11

wants to merge 20 commits into from

Conversation

cjayross
Copy link
Contributor

@cjayross cjayross commented Jul 7, 2019

Addresses #8

Please merge. Spare me of the shame that was ReplacementManager.

ReplacementManager was a global data structure that managed a dictionary between tensors that are represented symbolically and the arrays that represent the data they contain. It was a terrible idea to have this be defined globally as this causes lots of issues when using the same symbol for different tensors.

Now I simply have each tensor carry with it it's own dictionary entry to be used in the expand_tensor function. The function walks through an expression tree of tensor objects and adds these entries to a dictionary local to the function. This local dictionary is used instead of ReplacementManager.

@pep8speaks
Copy link

pep8speaks commented Jul 7, 2019

Hello @cjayross! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 251:81: E501 line too long (83 > 80 characters)

Comment last updated at 2019-07-09 05:24:41 UTC

@shreyasbapat
Copy link
Member

Squash the commits please!

@@ -1,5 +1,4 @@
"""
=======
Copy link
Member

Choose a reason for hiding this comment

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

No need to remove this!

@shreyasbapat
Copy link
Member

Rebase this!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants