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.

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

Jump to bottom

153 add new parameters tu mudirac in galaxy #154

Merged
merged 3 commits into from
Jul 8, 2024
Merged

153 add new parameters tu mudirac in galaxy #154

merged 3 commits into from
Jul 8, 2024

Conversation

subindev-d
Copy link
Contributor

added new parameters:

  • max_E_iter
  • max_nodes_iter
  • max_state_iter
  • uehling_lowcut
  • uehling_highcut
  • uehling_steps

@subindev-d subindev-d self-assigned this Jun 24, 2024
@subindev-d subindev-d linked an issue Jun 24, 2024 that may be closed by this pull request
Add new parameters tu MuDirac in Galaxy #153
Closed
patrick-austin
patrick-austin requested changes Jul 1, 2024
View reviewed changes
Copy link
Contributor

@patrick-austin patrick-austin left a comment

Choose a reason for hiding this comment

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

Most of these do not have physically valid or logical negative values (leading to unoptimised solutions or the core being dumped and the job failing). Mudirac appears not to validate this so we should. Regarding the choice of 0 for min, I did also consider whether 1 might be better but I suppose you may want to "skip" by setting it to zero even if this could/would lead to an unoptimised solution.

In all cases we should also be altering mudirac itself, I'll update muon-spectroscopy-computational-project/mudirac#15 as well. I'd hope that when that gets resolved, any decisions re: what lower values to accept can also be applied here.

mudirac/mudirac.xml Outdated Show resolved Hide resolved
mudirac/mudirac.xml Outdated Show resolved Hide resolved
mudirac/mudirac.xml Outdated Show resolved Hide resolved
mudirac/mudirac.xml Outdated Show resolved Hide resolved
mudirac/mudirac.xml Outdated Show resolved Hide resolved
@subindev-d @patrick-austin
code review changes
09f2ceb 
Co-authored-by: patrick-austin <61705287+patrick-austin@users.noreply.github.com>

Update mudirac/mudirac.xml

Co-authored-by: patrick-austin <61705287+patrick-austin@users.noreply.github.com>

Update mudirac/mudirac.xml

Co-authored-by: patrick-austin <61705287+patrick-austin@users.noreply.github.com>

Update mudirac/mudirac.xml

Co-authored-by: patrick-austin <61705287+patrick-austin@users.noreply.github.com>

Update mudirac/mudirac.xml

Co-authored-by: patrick-austin <61705287+patrick-austin@users.noreply.github.com>
@subindev-d subindev-d force-pushed the 153-add-new-parameters-tu-mudirac-in-galaxy branch from 99720a9 to 09f2ceb Compare July 2, 2024 12:14
@subindev-d subindev-d requested a review from patrick-austin July 2, 2024 13:47
@leandro-liborio
Copy link
Contributor

Hi both,

I cannot see these new parameters in MuDirac in the dev-server for Galaxy. When will you push this there?

@subindev-d subindev-d merged commit e697018 into dev Jul 8, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patrick-austin patrick-austin patrick-austin approved these changes

Assignees

@subindev-d subindev-d

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add new parameters tu MuDirac in Galaxy
3 participants
@subindev-d @leandro-liborio @patrick-austin