Skip to content

fix(mask): normalize Fmask FFT by cell volume to match Fprotein scale - #36

Merged
minhuanli merged 1 commit into
rs-station:masterfrom
DorisMai:fix-fmask-normalization
Jun 30, 2026
Merged

minhuanli merged 1 commit into
rs-station:masterfrom
DorisMai:fix-fmask-normalization

Conversation

@DorisMai

Copy link
Copy Markdown

Issue: realmask2Fmask used ifftn(norm="forward"), which makes Fsolvent practically grid size (sampling rate) dependent, and no longer on a comparable scale to Fprotein. We noticed this issue when making synthetic map which appeared dominated by Fsolvent contribution. When given experimental data, the off-scale issue was silently absorbed by the kmask scaling, but it also makes kmask less physically interpretable.

Proposed fix: use default ifftn normalization and scaled by cell volume so Fmask is (in theory) on the electron scale like Fprotein.

@minhuanli

Copy link
Copy Markdown
Collaborator

Hey Doris,

Thanks for the contribution!

Some people from Phenix raised this kmask scale issue when I first presented it to them, but it was ultimately set aside. As you mentioned, the constant scale can be handled by the scaling algorithms when they are initialized with true Fo datasets as references, and we have not spent much time working on synthetic dataset generation. In the cases where I experimented with synthetic data, I typically used the kmask scales obtained from fitting real datasets.

I think your change makes sense. I’ll run some tests on the R-free values for a few systems before merging it.

I’m also curious whether there is a physical interpretation of the kmask value. How do you usually choose or set it when generating synthetic datasets after addressing the constant scale issue?

Best,
Minhuan

@DorisMai

Copy link
Copy Markdown
Author

Hi Minhuan,
We are currently generating synthetic datasets with the default scaling values in SFC (e.g. kmask=0.35). I think kmask functions similarly to ksol, which for flat bulk solvent model should just be the average solvent density (electron/A^3), and 0.35 is pretty close to the pure water density value. This value yields a visually good synthetic map in phenix.fmodel (I set bsol=0 because I don't think SFC has an analogous parameter?) too.
I am curious to see if this affects Rfree, and happy to help look into additional issue further.

@minhuanli

Copy link
Copy Markdown
Collaborator

Nice, that is good to know, are you generating synthetic maps with SFC after you fixing the ifft constant scale?

For biso, I think it is the same as kiso in SFC, which is the isotropic solvent Bfactor, you can set here:

self.kisos = [

You can also set anisotropic solvent Bfactor at uaniso:

self.uanisos = [

@DorisMai

Copy link
Copy Markdown
Author

Yes I am currently doing one-off generation of synthetic maps for exploration after fixing the ifft constant. Ideally later for batch generation of more synthetic datasets I would prefer to use the official SFC release (a dependency of sampleworks now).

And for the kiso and uaniso, I think these are scaling for both Fprotein and Fsolvent in SFC, which matches the scale and b_cart in phenix.fmodel, but there is another b_sol just for the Fsolvent, so we need to be a bit careful for an exact comparison.

@DorisMai

Copy link
Copy Markdown
Author

I picked a few structures from PDB-REDO and the R-free (after scaling optimization) between master branch and this branch are very similar.

I am attaching

The command is
python dev/compare_fmask_rfactors.py benchmark_data/input.csv --data-dir benchmark_data --out benchmark_data/benchmark_results.csv
where PDB-REDO files were pre-downloaded into the benchmark_data folder.

@minhuanli minhuanli left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for the contribution Doris. Looks solid to me!

@minhuanli
minhuanli merged commit 783c260 into rs-station:master Jun 30, 2026
6 checks passed
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.

2 participants