Various naming convention changes in the engine #2769
Conversation
mahrud
force-pushed
the
refactor/engine
branch
from
15df2f7 to
59f1171
Compare
mahrud
changed the title
mahrud
changed the title
mahrud
force-pushed
the
refactor/engine
branch
from
56fd892 to
ed2988e
Compare
mahrud
changed the title
There was a problem hiding this comment.
@mahrud Thanks for teasing out the different kinds of monomials! I left a few comments, but will plan on merging this pull request after you take a look.
| const FreeModule *F, | ||
| vec &vmonom) const; | ||
| int moneq(const int *exp, int *m, const int *vars, int *exp2) const; | ||
| // vec strip_vector(vec &f, |
There was a problem hiding this comment.
I guess this means these 2 functions are not used any more...
There was a problem hiding this comment.
I somehow have no recollection of commenting this out, but yes you're right.
|
|
||
| int * ints() { return val.raw(); } | ||
| const int * ints() const { return val.raw(); } |
There was a problem hiding this comment.
I think the underlying representation is a const_varpower inside a std::vector?
There was a problem hiding this comment.
I don't want to touch this line here because the next PR changes it, but yes, I think you're right.
| @@ -427,7 +427,8 @@ MutableMatrix* ResF4toM2Interface::to_M2_MutableMatrix(SchreyerFrame& C, | |||
| Nterm** comps = newarray(Nterm*, nrows); | |||
| Nterm** last = newarray(Nterm*, nrows); | |||
|
|
|||
| int* m1 = M->make_one(); | |||
| monomial m1 = M->make_one(); | |||
| // FIXME: is exp a monomial or exponent vector? | |||
There was a problem hiding this comment.
It appears to be an exponent vector. I'm not sure why the length is nvars + 1... Probably it doesn't need to be larger than nvars.
There was a problem hiding this comment.
I agree, but maybe I'll leave this for you and Frank to resolve because a few lines down the to_expvector function
C.ring().monoid().to_expvector(w, exp, comp);Calls a method of ResMonoidDense in schreyer-resolution/res-moninfo-dense.hpp.
| @@ -15,51 +15,6 @@ typedef ntuple_monomials::Exponent ntuple_word; | |||
| typedef ntuple_word *ntuple_monomial; | |||
| typedef const ntuple_word *const_ntuple_monomial; | |||
|
|
|||
| template <> | |||
There was a problem hiding this comment.
I take it the code in this file was removed as it was not used, or is no longer used?
There was a problem hiding this comment.
I'm not sure if it was used (I suspect not), but they are all still covered by the template specialization, so you can still use them just as before.
|
@mahrud After you make the changes you mention, I'll merge it. Let me know when it is ready. |
mahrud
force-pushed
the
refactor/engine
branch
from
b488e1c to
e42fdc1
Compare
mahrud
force-pushed
the
refactor/engine
branch
from
e42fdc1 to
9018900
Compare
Once the tests pass I believe this is ready. For merging, try switching to "Rebase and merge". |
This PR is mostly just renames throughout the code, which unfortunately touch a lot of files:
I decided to keep this PR small, so the new
gc_vector<T>type and the templatedExponentList<T>type will be in the next PR, hopefully by then we have a better name for it.