Hi all,
First of all, thank you for maintaining bcftools and the mendelian2 plugin.
I may be misunderstanding the intended logic here, but while reviewing mendelian2.c I noticed what looks like a possible inconsistency between the -P/--ped and -p/--pfm code "paths" for sex-specific chrX handling, which looks like a possible bug for the -p "path" where the male and female case are fully swapped when parsing the input data.
From the source comments (e.g. line 88, line 111), the default/built-in mapping is described as:
- 0 = "1X" (male-pattern)
- 1 = "2X" (female-pattern)
Separately, two index constants (which as I understand are only supposed to be indices for parents in the relevant trio struct) are specified at lines 62-63 to be:
When parsing the input data, in the -P/--ped "path", sex appears to be handled as expected by parse_ped:
- PED sex==1 → lookup "1X"
- PED sex==2 → lookup "2X"
...which under the documented default mapping, would give:
- male → sex_id 0
- female → sex_id 1
Instead, in the -p/--pfm "path", there is no hash lookup, and the proband is prefixed explicitly by init_data, using those index constants mentioned above:
- 1X:Child,... sets sex_id = iDAD → 1
- 2X:Child,... sets sex_id = iMOM → 0
However, the help text says that -p accepts [1X:|2X:]P,F,M, with "1X:" for male pattern of chrX inheritance [2X:].
Unless I am missing something, this seems to imply that with the built-in/default rules:
-P male selects "1X", but
-p 1X: may instead select the "2X" rule set.
If that reading is correct, this would affect the handling of chrX Mendelian inconsistencies when using -p, especially in contexts where PAR/non-PAR behaviour depends on the selected sex-specific rule set - specifically, male and female seem to be swapped.
In fact, I have tested this on a small synthetic VCF example and I believe that I am seeing a confirmation of the issue when using -p. I am happy to share that if needed, but I thought that you could likely confirm this easily just by examining the relevant code.
Please let me know - and apologies in advance if I have misunderstood the intended mapping.
Thanks again for your work on the project!
Federico
Hi all,
First of all, thank you for maintaining bcftools and the mendelian2 plugin.
I may be misunderstanding the intended logic here, but while reviewing mendelian2.c I noticed what looks like a possible inconsistency between the
-P/--pedand-p/--pfmcode "paths" for sex-specific chrX handling, which looks like a possible bug for the-p"path" where the male and female case are fully swapped when parsing the input data.From the source comments (e.g. line 88, line 111), the default/built-in mapping is described as:
Separately, two index constants (which as I understand are only supposed to be indices for parents in the relevant trio struct) are specified at lines 62-63 to be:
When parsing the input data, in the
-P/--ped"path", sex appears to be handled as expected byparse_ped:...which under the documented default mapping, would give:
Instead, in the
-p/--pfm"path", there is no hash lookup, and the proband is prefixed explicitly by init_data, using those index constants mentioned above:However, the help text says that
-paccepts [1X:|2X:]P,F,M, with "1X:" for male pattern of chrX inheritance [2X:].Unless I am missing something, this seems to imply that with the built-in/default rules:
-Pmale selects "1X", but-p 1X:may instead select the "2X" rule set.If that reading is correct, this would affect the handling of chrX Mendelian inconsistencies when using
-p, especially in contexts where PAR/non-PAR behaviour depends on the selected sex-specific rule set - specifically, male and female seem to be swapped.In fact, I have tested this on a small synthetic VCF example and I believe that I am seeing a confirmation of the issue when using
-p. I am happy to share that if needed, but I thought that you could likely confirm this easily just by examining the relevant code.Please let me know - and apologies in advance if I have misunderstood the intended mapping.
Thanks again for your work on the project!
Federico