cbehan / pycftboot

A free frontend for the conformal bootstrap
MIT License
19 stars 10 forks source link

Processes per node to SDPB #19

Closed definelicht closed 4 years ago

definelicht commented 4 years ago

Apart from the -N flag to mpirun, SDPB itself seems to require --procsPerNode (as of version 2.3.1-3). Seems like this should be passed to SDPB also.

(based on branch subprocess)

cbehan commented 4 years ago

I don't think this is needed because self.options is always passed. If the sdpb_version is 2, then --procsPerNode will be part of that.

definelicht commented 4 years ago

This doesn't work for me, but I'll take another look at what's going on

definelicht commented 4 years ago

So here's what I found -- you're right that this PR treats a symptom, not the cause:

When running SDPB, self.options is empty, but sdpb_options is not:

-> os.spawnvp(os.P_WAIT, mpirun_path, ["mpirun", "-n", ppn, sdpb_path, "-s", name, "--precision=" + str(prec), "--findPrimalFeasible", "--findDualFeasible"] + self.options)
(Pdb) self.options
[]
(Pdb) self.get_option("procsPerNode")
'4'

I think the issue is that self.options does not fall back on defaults if they are not set. None of the options are set, but --procsPerNode results in an error, since this is a mandatory argument.

Is the intention that sdpb_options are used for all options that have not been explicitly set for the given instance?

cbehan commented 4 years ago

The original plan was to have self.options be empty until self.set_option() populates it. But this dates back to a time when there were no mandatory options.

Here's what a patch should do. The entry in sdpb_defaults should be changed from "4" to "0" and then every line that retrieves ppn should actually retrieve max(1, ppn).

definelicht commented 4 years ago

I'll leave this to you, so you can implement the behavior you want. Closing this issue, since it's not fixing the root cause.