Skip to content
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

Change default PDB chain ID from blank to 'Z' #747

Merged
merged 25 commits into from
Aug 22, 2019
Merged

Conversation

drroe
Copy link
Contributor

@drroe drroe commented Aug 16, 2019

@dacase Related to ParmEd/ParmEd#1055.

According to the PDB standard, the chain ID should be a "non-blank alphanumeric character". This PR makes it so that for output PDBs, if no chain ID information is present in the topology a default of 'Z' is used. Note that this does not happen when writing PQR files since those do not conform to the PDB standard anyway. The old behavior (outputting a blank character as chain ID) can be obtained via the already existing chainid <char> keyword for PDB trajout, e.g.

trajout mypdb.pdb chainid ' '

This PR also adds a new keyword to the mask command, trajargs, which allows passing a comma-separated list of trajectory arguments to mask when writing out Mol2/PDB files (which allows recovering the old PDBs with blank chain ID for that command).

Updates docs (including fixing areas where text ran off the PDB page) and adds a test.

Daniel R. Roe added 18 commits August 15, 2019 09:48
… use the default chain id when writing. This makes it so that e.g. selection by chainid wont return a false positive when specifying the default chain id, printing the topology wont make it seem like the default chain ID is actually part of the topology, etc.
@drroe drroe self-assigned this Aug 16, 2019
@drroe
Copy link
Contributor Author

drroe commented Aug 19, 2019

@swails from Jenkins:

./configure: line 355: pgc++: command not found
script returned exit code 1

Are PGI compilers no longer available or is this a config change thing?

@swails
Copy link
Contributor

swails commented Aug 21, 2019

run jenkins

1 similar comment
@swails
Copy link
Contributor

swails commented Aug 21, 2019

run jenkins

@swails
Copy link
Contributor

swails commented Aug 21, 2019

The problem was that the PGI installation was broken in Docker containers on the Jenkins master (it works everywhere else). The reason was a shortcut I took on that node -- I don't have enough space on my root disk to fit all of the compiler suites I want to install in /opt, so I installed them in /iscratch/opt and created a bind mount from /iscratch/opt/pgi to /opt/pgi, which is where it is installed in all of the other slave nodes (and where the Dockerfile tries to mount it from).

But in a docker container that only mounts /opt/pgi, links to /iscratch/opt/pgi all break (which is why PGI can't be found).

For the time being I just removed the PGI label from the master node so Jenkins won't try PGI builds on that node. Once I get the PGI installation fixed on that node I'll add the label back. But for now, Jenkins is green again :)

@drroe
Copy link
Contributor Author

drroe commented Aug 21, 2019

But for now, Jenkins is green again :)

@swails Thanks Jason!

I'm going to try one more appveyor build. Right now the issue is:

  CPPTRAJ: Ewald test (ortho), 10 frames
../MasterTest.sh: line 480: /dev/stderr: No such file or directory
../MasterTest.sh: line 450: /dev/stderr: No such file or directory
  5 out of 5 executions exited with an error.
  5 out of 5 comparisons failed.
../MasterTest.sh: line 82: /dev/stdout: No such file or directory
../MasterTest.sh: line 83: /dev/stdout: No such file or directory

In lines 82/83 I'm just using the standard POSIX redirects (> and >>) which have always worked on appveyor before. If they're somehow broken I may drop support for appveyor entirely. I'd be interested to know how many people are actually building cpptraj on windows anyway - my guess is a very small fraction.

@hainm
Copy link
Contributor

hainm commented Aug 22, 2019

User can get linux subsystem fro Microsoft store now: https://docs.microsoft.com/en-us/windows/wsl/install-win10

:))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants