Adding support for direct problem loading#80
Conversation
no more anything no more class drop import add back bring back underscore debug changing responsibility adding a getter add axes new numpy conv new way bug making rows hopefully final new way final updates dont flip back Revert "dont flip back" This reverts commit 449077d.
|
@jvkersch Let me know what you think about this |
jvkersch
left a comment
There was a problem hiding this comment.
@supersimple33 Thank you for your contribution, this looks very promising!
Just a few comments inline (in general, it is good to keep the PR limited to the actual change -- stylistic changes are preferably done separately). Also, could you add a unit test to exercise this functionality
| def adj(self): | ||
| cdef int[:, :] adj_data | ||
| if self.initialized and self.c_data.adj != NULL: | ||
| adj_data = np.zeros((self.ncount, self.ncount), dtype=np.int32) |
There was a problem hiding this comment.
This can be np.empty -- you're filling in the entries of the array later on anyway, so no need to zero initialize it now.
| for i in range(self.ncount): | ||
| for j in range(self.ncount): | ||
| if i > j: | ||
| adj_data[i, j] = self.c_data.adj[i][j] | ||
| elif i < j: | ||
| adj_data[i, j] = self.c_data.adj[j][i] | ||
| else: | ||
| adj_data[i, j] = 0 |
There was a problem hiding this comment.
I think this can be done a little more tightly:
for i in range(self.ncount):
adj_data[i, i] = 0
for j in range(i + 1, self.ncount):
adj_data[i, j] = adj_data[j, i] = self.c_data.adj[i, j]Please double-check though...
| cdef _CCdatagroup dat | ||
|
|
||
| dat = _CCdatagroup() |
There was a problem hiding this comment.
Can we combine this as cdef _CCdatagroup dat = _CCdatagroup()?
|
|
||
| solution = Solution.from_file(tmp / "problem.sol", output=res.stdout) | ||
| return solution | ||
| return Solution.from_file(tmp / "problem.sol", output=res.stdout) |
There was a problem hiding this comment.
This is not related to the current PR, it would be better to revert this. (topical diffs make for better traceability in the git history)
| if pyconcorde_binaries.exists(): | ||
| # Git checkout, with pyconcorde-build as git subtree | ||
| location = _PLATFORM_MAP[(platform.system(), platform.machine())] | ||
| concorde_exe = pyconcorde_binaries / location / "concorde" | ||
| else: | ||
| if not pyconcorde_binaries.exists(): |
There was a problem hiding this comment.
This is not related to the current PR, it would be better to revert this.
| concorde_exe = project_dir / "concorde" / "concorde" | ||
| return concorde_exe | ||
| return project_dir / "concorde" / "concorde" | ||
| # Git checkout, with pyconcorde-build as git subtree | ||
| location = _PLATFORM_MAP[(platform.system(), platform.machine())] | ||
| return pyconcorde_binaries / location / "concorde" |
There was a problem hiding this comment.
This is not related to the current PR, it would be better to revert this.
| 'CFLAGS="{cflags}" ./configure --prefix {data} ' | ||
| "--with-qsopt={data} {flags}" | ||
| ).format(cflags=cflags, data=datadir, flags=flags) | ||
| cwd = f'CFLAGS="{cflags}" ./configure --prefix {datadir} --with-qsopt={datadir} {flags}' |
There was a problem hiding this comment.
This is not related to the current PR, it would be better to revert this.
Hey I had a use case where I needed to skip the file-saving step of the process and so I wrote this PR which allows one to leverage the tri2dat function of Concorde via this python module.
This is just #77 but with its dedicated branch now