Skip to content

Fold the exclusions of scalars from the hestenes dot product into the table#134

Merged
utensil merged 1 commit intopygae:masterfrom
eric-wieser:remove-hestenes-special-case
Dec 6, 2019
Merged

Fold the exclusions of scalars from the hestenes dot product into the table#134
utensil merged 1 commit intopygae:masterfrom
eric-wieser:remove-hestenes-special-case

Conversation

@eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Dec 4, 2019

Split from an earlier PR (#99) that failed CI. This part strikes me as the part that wasn't wrong.

It's not particularly useful, but this now means that ga.dot_table_dict[S(1), S(1)] now returns 0 and not 1.

This does not affect the output of the hestenes_dot function, it just brings the table in line with it.

@eric-wieser

This comment has been minimized.

@eric-wieser eric-wieser force-pushed the remove-hestenes-special-case branch from efff455 to e1b1c92 Compare December 4, 2019 16:16
@eric-wieser eric-wieser force-pushed the remove-hestenes-special-case branch 2 times, most recently from c8550c8 to 108519a Compare December 5, 2019 10:11
@eric-wieser eric-wieser force-pushed the remove-hestenes-special-case branch from 108519a to b1bbc27 Compare December 5, 2019 14:43
@codecov
Copy link

codecov bot commented Dec 5, 2019

Codecov Report

Merging #134 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #134      +/-   ##
==========================================
+ Coverage   67.53%   67.54%   +<.01%     
==========================================
  Files           8        8              
  Lines        4796     4794       -2     
==========================================
- Hits         3239     3238       -1     
+ Misses       1557     1556       -1
Impacted Files Coverage Δ
galgebra/ga.py 75.09% <100%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ea8eea...e301aee. Read the comment docs.

@eric-wieser eric-wieser force-pushed the remove-hestenes-special-case branch from b1bbc27 to 945c18d Compare December 5, 2019 15:00
@pygae pygae deleted a comment from lgtm-com bot Dec 5, 2019
@pygae pygae deleted a comment from lgtm-com bot Dec 5, 2019
@eric-wieser eric-wieser changed the base branch from master to 0.4.4-release December 6, 2019 13:37
@eric-wieser eric-wieser changed the base branch from 0.4.4-release to master December 6, 2019 13:37
… table

This means that `ga.dot_table_dict[S(1), S(1)] == S(0)`, whereas previously it was incorrectly 1.
@eric-wieser eric-wieser force-pushed the remove-hestenes-special-case branch from 945c18d to e301aee Compare December 6, 2019 13:39
Comment on lines -96 to -98
if (isinstance(expr1, numbers.Number) or expr1.is_commutative) \
or (isinstance(expr2, numbers.Number) or expr2.is_commutative):
return expr1 * expr2
Copy link
Member Author

Choose a reason for hiding this comment

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

This part was actually incorrect for for when mul_dict[S(1), S(1)] != S(1), like it is for the hestenes dot

Copy link
Member Author

Choose a reason for hiding this comment

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

I suppose strictly this change could go in all by itself

@eric-wieser eric-wieser requested a review from utensil December 6, 2019 13:52
Copy link
Member

@utensil utensil left a comment

Choose a reason for hiding this comment

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

It took me some time to map the diff to formulas.

@utensil utensil merged commit 159cc2a into pygae:master Dec 6, 2019
@eric-wieser eric-wieser deleted the remove-hestenes-special-case branch December 8, 2019 10:56
@utensil utensil added this to the 0.4.5 milestone Dec 18, 2019
@eric-wieser eric-wieser added the component: core Ga, Mv, Metric, etc label Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: core Ga, Mv, Metric, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants