-
Notifications
You must be signed in to change notification settings - Fork 64
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
Features from meuns/galgebra: GA4CS examples, PGA, code-generation, and more #68
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
@meuns Can you make a minimal reproduction of this problem? |
This is quite easy to reproduce. Just pass any Mv instance to simplify. If I remember well the dot product, the left and right contractions returned some sympy scalar expressions and now they return scalar Mv instances. Not sure this is a problem once the user knows. |
That sounds like the issue I filed the other day then, #53 |
This comment has been minimized.
This comment has been minimized.
From this description, I can barely recalled that I have done something similar intentionally for consistency. Can't be sure until I get down to try a reprodution. |
@@ -700,7 +703,37 @@ def _build_basis_blade_symbol(self, base_index): | |||
raise ValueError('!!!!No unique root symbol for wedge_print = False!!!!') | |||
return Symbol(symbol_str, commutative=False) | |||
|
|||
def _build_bases(self): | |||
def build_cobases(self, coindexes=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
xref #215.
I think I understand the rough intent here, although it's not clear to me why a separate set of indices or signs are needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a "canonical" way for building cobases (see John Browne's book) and PGA uses some kind of "tricky" way for building it (J and Jinv stuff). My implementation is probably not the best we can do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@enkimute, is "cobase" consistent with the terminology you'd use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sign is for using the orthogonal wedge implementation provided by galgebra which needs a canonical basis. PGA basis isn't canonical too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you say the PGA basis isn't canonical, do you mean that the default is not to use lexicographically-order basis blades? Eg, e132
is considered a "base" blade in that algebra, rather than e123
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This forum post captures the issue I assume?
https://discourse.bivector.net/t/dual-operator-disambiguation/59/45
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another way to handle this might just be in the printing only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be very nice if you are able to run test_pga.py without adding the custom indexes and signs. I don't see how we can achieve this.
@staticmethod | ||
def _make_blade(ga, __name, __grade, **kwargs): | ||
if isinstance(__name, str) and isinstance(__grade, int): | ||
return reduce(Mv.__xor__, [ga.mv('%s%d' % (__name, i), 'vector') for i in range(__grade)], | ||
ga.mv(1, 'scalar')).obj | ||
else: | ||
raise TypeError("name must be a string and grade must be an int") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nice idea, but unfortunately it won't work with ga.make_grad(some_blade)
, as the coefficients are no longer single variables.
Perhaps it's worth cherry-picking into master anyway.
else: | ||
for blade in blade_lst: | ||
if not blade.is_base() or not blade.is_blade(): | ||
raise ValueError("%s expression isn't a basis blade" % blade) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This got upstreamed, thanks!
Hi @utensil,
I found a weird thing using pygae/galgebra. Some methods return a Mv instance now but a sympy expression is enough and this crashes Sympy 1.3. Sympy just goes into an infinite recursion using its reflection system and finish by raising an exception.
Regards