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

Wall loss example #424

Merged
merged 17 commits into from
Jan 5, 2024
Merged

Wall loss example #424

merged 17 commits into from
Jan 5, 2024

Conversation

Gorkowski
Copy link
Collaborator

@Gorkowski Gorkowski commented Jan 2, 2024

Fixes #402

  • adding rectangular beta0
  • testing rectangular code
  • example of different losses

@Gorkowski Gorkowski marked this pull request as draft January 2, 2024 21:44
Co-authored-by: Sourcery AI <>
@Gorkowski
Copy link
Collaborator Author

Gorkowski commented Jan 2, 2024

@ngmahfouz
I wrote things out more explicitly in wlc, in this update too

I was going, to update wlc name too, but that might have side-effects that need to be fixed at that point from other dependencies.

Copy link

github-actions bot commented Jan 2, 2024

PR Preview Action v1.4.6
🚀 Deployed preview to https://uncscode.github.io/particula/pr-preview/pr-424/
on branch gh-pages at 2024-01-04 22:58 UTC

@ngmahfouz
Copy link
Collaborator

@Gorkowski do you want me to fix the initial mess here? I put the initial function as a placeholder and I left it half-baked. Let me look into it and rewrite the whole thing so that it is more flexible and generic and can be called from anywhere

@ngmahfouz
Copy link
Collaborator

I can also write a notebook with references and visualizations, etc.

Copy link
Collaborator Author

@Gorkowski Gorkowski left a comment

Choose a reason for hiding this comment

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

@ngmahfouz Yeah, feel free to refactor and add to this PR. I was thinking of protyping out the wall loss forward simulation.

After that work on the data fitting as a separate PR.

@Gorkowski
Copy link
Collaborator Author

Gorkowski commented Jan 4, 2024

@ngmahfouz Here is my first cut of the chamber forward simulations. Look at the plots in the notebook to double-check the dynamics look right.

Also, some negative concentration values slip in at high concentrations of 1e5 and smaller particle sizes. This is due to the ODE taking too large of a step (i think). Ideas on fixing that would be good :) It's not a significant problem, but something that a novice person could find puzzling if they don't understand it.

@Gorkowski Gorkowski marked this pull request as ready for review January 4, 2024 22:49
@ngmahfouz
Copy link
Collaborator

@ngmahfouz Here is my first cut of the chamber forward simulations. Look at the plots in the notebook to double-check the dynamics look right.

Also, some negative concentration values slip in at high concentrations of 1e5 and smaller particle sizes. This is due to the ODE taking too large of a step (i think). Ideas on fixing that would be good :) It's not a significant problem, but something that a novice person could find puzzling if they don't understand it.

Will review tonight; we can set a a number limiter to ensure it never goes negative ;)

Copy link
Collaborator

@ngmahfouz ngmahfouz left a comment

Choose a reason for hiding this comment

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

Good to go! Really nice work. You're making me excited about this work again :)

I will do my best to dedicate at least a week of January (likely last one) on particula ..... sorry too much instability in my life!

@ngmahfouz
Copy link
Collaborator

Screenshot 2024-01-04 at 11 34 36 PM

Not sure if you see this, but I would encourage you to enable it if you'd like; if a reviewer (me) approves the PR, then it will automatically merge!

@Gorkowski Gorkowski enabled auto-merge January 5, 2024 13:02
@Gorkowski Gorkowski added this pull request to the merge queue Jan 5, 2024
Merged via the queue into main with commit 4d9baee Jan 5, 2024
8 checks passed
@Gorkowski Gorkowski deleted the Gorkowski/issue402 branch January 5, 2024 13:37
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.

Chamber Wall Loss Scoping Study
2 participants