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

[FIX] error in conformer model #72

Merged
merged 13 commits into from
Jan 24, 2024
Merged

[FIX] error in conformer model #72

merged 13 commits into from
Jan 24, 2024

Conversation

jcharkow
Copy link
Collaborator

@jcharkow jcharkow commented Jan 8, 2024

conformer model expected properties of transition group that were not present. Change UI such that require a spectral library to get the library intensities.

TODO: Fix breaking changes to the UI side

Contents (#72)

Other

  • Roestlab/massseer into patch/conformer
  • Roestlab/massseer into patch/conformer
  • address Justin's comments
  • rename method pad-->adjust_length
  • change mob/chrom to spectrum
  • remove unnecessary imports

Uncategorised!

  • [FIX] error in conformer model
  • [FIX] get Conformer peak picker working with GUI
  • [FIX] conformer peak picker with streamlit
  • [FIX] conformer of mulitple lengths
  • update README.md
  • [MINOR] refactor
  • [FEATURE] auto window size determining

conformer model expected properties of transition group that were not
present. Require a spectral library to get the library intensities
@jcharkow jcharkow requested a review from singjc January 24, 2024 14:50
Copy link
Collaborator

@singjc singjc left a comment

Choose a reason for hiding this comment

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

Looks good! Just a few minor changes and comments

@@ -73,8 +76,9 @@ def load_model(self):
raise ImportError("onnxruntime is required for loading the pretrained Conformer model, but not installed.")
# Load pretrained model
self.onnx_session = onnxruntime.InferenceSession(self.pretrained_model_file)
self.window_size = self.onnx_session.get_inputs()[0].shape[2]
Copy link
Collaborator

@singjc singjc Jan 24, 2024

Choose a reason for hiding this comment

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

Can you wrap this in an exception handling to assert that get_inputs() must have a value (non-empty list) and that the first input must be a 3 dimensional array.

rename pad to adjust_length because the length can either be padded or
truncated
def adjust_length(self, length: int) -> 'Spectrum':
"""
Adjust the length of the mobilogram to a given length, this involved either padding or truncating the chromatogram
Copy link
Collaborator

Choose a reason for hiding this comment

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

mobilogram/chromatogram -> spectrum

@jcharkow jcharkow merged commit 9320cc6 into dev Jan 24, 2024
10 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.

3 participants