str_without returning Varkey

[1ozturkbe]

I am running tests in robust that I am trying to update to python3, and realized that there are cases when calling Varkey.str_without() returns Varkey! This breaks robust tests on line 42 of gpkit.varkey.py:

        fullstr = self.str_without(["modelnums", "vec"])
        self.eqstr = fullstr + str(self.lineage) + self.unitrepr

when fullstr is actually not a string. Trying to figure out what is going on rn, lmk if you have an idea @bqpd.

[whoburg]

Would be great to have a minimum working example that can be turned into a test. Ideally without dependencies.

[1ozturkbe]

Alright... I think I know why this happens, now that I have spent most of today dissecting it... This occurs when I am creating a Monomial in robust.

        first_monomial = Monomial(self.p.exps, self.p.cs)

where I am passing a dict (self.p.exps) of {Varkey: float} describing the exponents, and a float coefficient. Since Monomial inherits from Signomial, the error occurs here in Signomial:

            elif isinstance(hmap, dict):
                exp = HashVector({VarKey(k): v
                                  for k, v in hmap.items() if v})
                hmap = NomialMap({exp: mag(cs)})
                hmap.units_of_product(cs)

It understands that the map of exponents is a dict, then GPkit proceeds to try to create a HashVector with VarKey(k) as keys, where k is a VarKey! I am super confused why GPkit does this. Can you explain @bqpd?

[1ozturkbe]

So from the look of the implementation in GPkit tests, it seems like the dict of exps passed onto Monomial has keys that are varkey strings, not VarKey objects. Will set up a fix.

[whoburg]

@1ozturkbe I suggest creating a separate issue for the Monomial __init__ discussion... in part because I think it will help us to refine our unit tests for that interface. Would be great to have MWE's for both that issue and this issue.

[1ozturkbe]

This is referenced in pull request #1419.

[bqpd]

You can't create monomials like that anymore...since like a year ago? pass it dict(zip(exps, cs))

[bqpd]

oh hmm nvm

[bqpd]

So. I don't understand this method of creating a copy of self.p: why not just use self.p itself? But if you really need to, don't recreate the NomialMap from scratch: pass it Monomial(self.p.hmap)

[whoburg]

@bqpd quick process question -- why is this closed? It seems like a bug (would be better with a MWE, but still a bug). I would have thought we would leave this open until either

• a test is added that currently fails for this case, or
• the documentation and unit tests are updated to clarify that creating a Monomial in this way is not allowed.

What's your thought process wise?

[bqpd]

ah I left the note for that over in #1419 : "I think the right response here might in fact be to remove dict-based creation of Signomials; it's only ever done in tests, and I wouldn't want users to do it (even for libraries like robust) bc it's slower and more prone to error than using NomialMaps or operator overloading."

[whoburg]

gotcha, totally fair solution. In that case just want to also track the desire for a unit test that fails until dict-based creation of Signomials is removed

[1ozturkbe]

Here is why I was initializing new monomials. I wanted to separate the monomials within that posynomial, and there doesn't seem to be an easy way to do that without initializing new monomials through chopping up into self.p.exps[i], for each ith monomial.

[bqpd]

Oh hmm that is a good usecase

[bqpd]

I'd recommend

monmaps = [NomialMap({exp: c}) for exp, c in self.p.hmap.items()]
for monmap in monmaps:
    monmap.units = self.p.hmap.units
mons = [Monomial(monmap) for monmap in monmaps]

sorry for the annoying stuff with units, but this should be (a little) faster.

[1ozturkbe]

Cool, I'll do that instead.

[1ozturkbe]

That being said, it might be nice to have a method to retrieve monomials (Posynomial.chop hehe), especially since we will need to do this a lot to use Riley's solution method.