Skip to content

Adapt default charge change score to allow for charge changing transformations#83

Merged
IAlibay merged 8 commits intomainfrom
change_charge_score_default
Nov 17, 2025
Merged

Adapt default charge change score to allow for charge changing transformations#83
IAlibay merged 8 commits intomainfrom
change_charge_score_default

Conversation

@hannahbaumann
Copy link
Contributor

@hannahbaumann hannahbaumann commented Sep 19, 2025

Only adapt this in gufe bindings to not affect main Lomap behaviour.

  • Check if docs have to be updated
    ---> The scorers are not part of the API documentation?!

Copy link
Contributor

@atravitz atravitz left a comment

Choose a reason for hiding this comment

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

This should have a corresponding test and news entry, otherwise looks good!

Copy link
Member

@IAlibay IAlibay left a comment

Choose a reason for hiding this comment

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

Hold on my end - I want to double check the other ways to use Lomap networks aren't being affected.

@hannahbaumann
Copy link
Contributor Author

@atravitz I changed a test that was checking the connection of networks when using a charge score of 0.1 to test that with the new default (charge score of 0.1) we are getting connected networks. Or should I also explicitly test for the value of the score?
I also added a news item.

@atravitz
Copy link
Contributor

The check you have is great, but I would also explicitly check that the correct value is being assigned, since we'd want tests to break if the actual value is getting mutated somewhere and turning out to be not 0.1.

Other than that, this looks good to me!

Copy link
Member

@IAlibay IAlibay left a comment

Choose a reason for hiding this comment

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

Sorry about the delay, took me a while to realise all of scorers.py is a duplicate of what's in the MCS file (we should fix that later).

hannahbaumann and others added 2 commits November 6, 2025 09:17
Co-authored-by: Alyssa Travitz <31974495+atravitz@users.noreply.github.com>
@hannahbaumann
Copy link
Contributor Author

Thanks! @atravitz I added a test for the new default charge change score!

@atravitz
Copy link
Contributor

atravitz commented Nov 6, 2025

one more thing - does this need to be reflected in the docs somewhere?

@hannahbaumann
Copy link
Contributor Author

@atravitz : I added the default_lomap_score function to the API docs built, so the updated default should be visible there. Docs are however currently failing for this repo.

Copy link
Contributor

@atravitz atravitz left a comment

Choose a reason for hiding this comment

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

thanks @hannahbaumann - the docs are building correctly for me locally, so I consider this a readthedocs headache that I will address in a follow-up (likely here).

@atravitz
Copy link
Contributor

docs are fixed!

@IAlibay IAlibay merged commit d65bd19 into main Nov 17, 2025
11 checks passed
@IAlibay IAlibay deleted the change_charge_score_default branch November 17, 2025 22:46
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.

Change the default charge score in gufe bindings to allow for charge changing transformations in Lomap networks

3 participants