Skip to content

Mzml danger #152

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.

Sign up for GitHub

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

Open
cbielow wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Mzml danger #152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a95e23d
speed fixes to mzML parsing, in particular avoid double lookup of CV …
cbielow Feb 24, 2025
8ee428a
dangerous commit -- not correct
cbielow Feb 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/openms/include/OpenMS/FORMAT/HANDLERS/XMLHandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,9 @@ namespace OpenMS
// Converts from a wide-character string to a narrow-character string.
inline static String toNative_(const XMLCh* str)
{
return String(unique_xerces_ptr<char>(xercesc::XMLString::transcode(str)).get());
String r;
appendASCII(str, xercesc::XMLString::stringLen(str), r);
return r;
Comment on lines +228 to +230
Copy link
Owner Author

@cbielow cbielow Feb 24, 2025

Choose a reason for hiding this comment

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

this change yields about 40% higher read speeds for mzml, at the cost of correctness for non-ascii encodings....

Copy link
Owner Author

@cbielow cbielow Feb 24, 2025

Choose a reason for hiding this comment

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

103 mb/sec --> 142 mb/sec

}

// Converts from a wide-character string to a narrow-character string.
Expand Down
44 changes: 23 additions & 21 deletions src/openms/source/FORMAT/HANDLERS/MzMLHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,25 +692,27 @@ namespace OpenMS::Internal
constexpr XMLCh s_external_spectrum_id[] = { 'e','x','t','e','r','n','a','l','S','p','e','c','t','r','u','m','I','D' , 0};
// constexpr XMLCh s_default_source_file_ref[] = { 'd','e','f','a','u','l','t','S','o','u','r','c','e','F','i','l','e','R','e','f' , 0};
constexpr XMLCh s_scan_settings_ref[] = { 's','c','a','n','S','e','t','t','i','n','g','s','R','e','f' , 0};
String tag = sm_.convert(qname);
open_tags_.push_back(tag);


open_tags_.push_back(sm_.convert(qname));
const String& tag = open_tags_.back();

// do nothing until a spectrum/chromatogram/spectrumList ends
if (skip_spectrum_ || skip_chromatogram_)
{
return;
}

//determine parent tag
String parent_tag;
// determine parent tag
const String* parent_tag = &tag; // set to some valid string
if (open_tags_.size() > 1)
{
parent_tag = *(open_tags_.end() - 2);
parent_tag = &(*(open_tags_.end() - 2));
}
String parent_parent_tag;
const String* parent_parent_tag = &tag; // set to some valid string
if (open_tags_.size() > 2)
{
parent_parent_tag = *(open_tags_.end() - 3);
parent_parent_tag = &(*(open_tags_.end() - 3));
}

if (tag == "spectrum")
Expand Down Expand Up @@ -859,21 +861,21 @@ namespace OpenMS::Internal
}
else if (tag == "cvParam")
{
String value = "";
String value;
optionalAttributeAsString_(value, attributes, s_value);
String unit_accession = "";
String unit_accession;
optionalAttributeAsString_(unit_accession, attributes, s_unit_accession);
handleCVParam_(parent_parent_tag, parent_tag, attributeAsString_(attributes, s_accession), attributeAsString_(attributes, s_name), value, unit_accession);
handleCVParam_(*parent_parent_tag, *parent_tag, attributeAsString_(attributes, s_accession), attributeAsString_(attributes, s_name), value, unit_accession);
}
else if (tag == "userParam")
{
String type = "";
String type;
optionalAttributeAsString_(type, attributes, s_type);
String value = "";
String value;
optionalAttributeAsString_(value, attributes, s_value);
String unit_accession = "";
String unit_accession;
optionalAttributeAsString_(unit_accession, attributes, s_unit_accession);
handleUserParam_(parent_parent_tag, parent_tag, attributeAsString_(attributes, s_name), type, value, unit_accession);
handleUserParam_(*parent_parent_tag, *parent_tag, attributeAsString_(attributes, s_name), type, value, unit_accession);
}
else if (tag == "referenceableParamGroup")
{
Expand Down Expand Up @@ -944,7 +946,7 @@ namespace OpenMS::Internal
String ref = attributeAsString_(attributes, s_ref);
for (Size i = 0; i < ref_param_[ref].size(); ++i)
{
handleCVParam_(parent_parent_tag, parent_tag, ref_param_[ref][i].accession, ref_param_[ref][i].name, ref_param_[ref][i].value, ref_param_[ref][i].unit_accession);
handleCVParam_(*parent_parent_tag, *parent_tag, ref_param_[ref][i].accession, ref_param_[ref][i].name, ref_param_[ref][i].value, ref_param_[ref][i].unit_accession);
}
}
else if (tag == "scan")
Expand Down Expand Up @@ -5226,7 +5228,7 @@ namespace OpenMS::Internal
}
Base64::encodeIntegers(data64_to_encode, Base64::BYTEORDER_LITTLEENDIAN, encoded_string, options_.getCompression());

String data_processing_ref_string = "";
String data_processing_ref_string ;
if (!array.getDataProcessing().empty())
{
data_processing_ref_string = String("dataProcessingRef=\"dp_sp_") + s + "_bi_" + m + "\"";
Expand Down Expand Up @@ -5256,7 +5258,7 @@ namespace OpenMS::Internal
for (Size p = 0; p < array.size(); ++p)
data_to_encode[p] = array[p];
Base64::encodeStrings(data_to_encode, encoded_string, options_.getCompression());
String data_processing_ref_string = "";
String data_processing_ref_string ;
if (!array.getDataProcessing().empty())
{
data_processing_ref_string = String("dataProcessingRef=\"dp_sp_") + s + "_bi_" + m + "\"";
Expand Down Expand Up @@ -5426,7 +5428,7 @@ namespace OpenMS::Internal
// Try and identify whether we have a CV term for this particular array (otherwise write the array name itself)
ControlledVocabulary::CVTerm bi_term = getChildWithName_("MS:1000513", array.getName()); // name: binary data array

String unit_cv_term = "";
String unit_cv_term ;
if (array_metadata.metaValueExists("unit_accession"))
{
ControlledVocabulary::CVTerm unit = cv_.getTerm(array_metadata.getMetaValue("unit_accession"));
Expand All @@ -5449,7 +5451,7 @@ namespace OpenMS::Internal
np_config = pf_options_.getNumpressConfigurationFloatDataArray();
}

String data_processing_ref_string = "";
String data_processing_ref_string ;
if (!array.getDataProcessing().empty())
{
data_processing_ref_string = String("dataProcessingRef=\"dp_sp_") + spec_chrom_idx + "_bi_" + array_idx + "\"";
Expand Down Expand Up @@ -5598,7 +5600,7 @@ namespace OpenMS::Internal
data64_to_encode[p] = array[p];
}
Base64::encodeIntegers(data64_to_encode, Base64::BYTEORDER_LITTLEENDIAN, encoded_string, options_.getCompression());
String data_processing_ref_string = "";
String data_processing_ref_string ;
if (!array.getDataProcessing().empty())
{
data_processing_ref_string = String("dataProcessingRef=\"dp_sp_") + c + "_bi_" + m + "\"";
Expand Down Expand Up @@ -5630,7 +5632,7 @@ namespace OpenMS::Internal
data_to_encode[p] = array[p];
}
Base64::encodeStrings(data_to_encode, encoded_string, options_.getCompression());
String data_processing_ref_string = "";
String data_processing_ref_string ;
if (!array.getDataProcessing().empty())
{
data_processing_ref_string = String("dataProcessingRef=\"dp_sp_") + c + "_bi_" + m + "\"";
Expand Down
31 changes: 14 additions & 17 deletions src/openms/source/FORMAT/HANDLERS/XMLHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,27 +204,14 @@ namespace OpenMS::Internal
DataValue cv_value = value;

// Abort on unknown terms
if (!cv.exists(accession))
try
{
// in 'sample' several external CVs are used (Brenda, GO, ...). Do not warn then.
if (parent_tag != "sample")
{
warning(LOAD, String("Unknown cvParam '") + accession + "' in tag '" + parent_tag + "'.");
return DataValue::EMPTY;
}
}
else
{
const ControlledVocabulary::CVTerm& term = cv.getTerm(accession);
const ControlledVocabulary::CVTerm& term = cv.getTerm(accession); // throws Exception::InvalidValue if missing

// check if term name and parsed name match
if (name != term.name)
{
const String parsed_name = String(name).trim();
const String correct_name = String(term.name).trim();
if (parsed_name != correct_name)
{
warning(LOAD, String("Name of CV term not correct: '") + term.id + " - " + parsed_name + "' should be '" + correct_name + "'");
}
warning(LOAD, String("Name of CV term not correct: '") + term.id + " - " + name + "' should be '" + term.name + "'");
}
if (term.obsolete)
{
Expand Down Expand Up @@ -322,6 +309,15 @@ namespace OpenMS::Internal
return DataValue::EMPTY;
}
}
catch (const Exception::InvalidValue& /*e*/)
{
// in 'sample' several external CVs are used (Brenda, GO, ...). Do not warn then.
if (parent_tag != "sample")
{
warning(LOAD, String("Unknown cvParam '") + accession + "' in tag '" + parent_tag + "'.");
return DataValue::EMPTY;
}
}

if (!unit_accession.empty())
{
Expand Down Expand Up @@ -437,6 +433,7 @@ namespace OpenMS::Internal
// and all bytes except the least significant one will be zero. Thus
// we can convert to char directly (only keeping the least
// significant byte).

const XMLCh* it = chars;
const XMLCh* end = it + length;

Expand Down
Loading