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

Several updates to the Micro Manager config files #609

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

Conversation

IshaanDesai
Copy link
Member

@IshaanDesai IshaanDesai commented Jan 22, 2025

This PR updates the following things in the Micro Manager config file:

  1. Change the attribute config_file_name to precice_config_file_name to clearly differentiate between the Micro Manager config and the preCICE config files. Introduced in #133
  2. Removing the scalar and vector values to data name keys in the Micro Manager config files as they are no longer necessary since #142

Additionally, the requirements.txt in two-scale-heat-conduction now points to the develop branch of the Micro Manager repository, as the changes above are not in a release.

Checklist:

  • I added a summary of any user-facing changes (compared to the last release) in the changelog-entries/<PRnumber>.md.
  • I will remember to squash-and-merge, providing a useful summary of the changes of this PR.

@IshaanDesai IshaanDesai requested a review from uekerman January 22, 2025 10:03
Copy link
Member

@uekerman uekerman 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 to me, but I have not tested it.

@MakisH Should we document the version dependency somewhere? (We now require the newest develop branch of the Micro Manager)

changelog-entries/609.md Outdated Show resolved Hide resolved
Co-authored-by: Benjamin Uekermann <[email protected]>
@IshaanDesai
Copy link
Member Author

Should we document the version dependency somewhere? (We now require the newest develop branch of the Micro Manager)

Currently we have the Micro Manager version in the requirements.txt of the respective micro participants. There should be a way to directly define the develop branch as a version. I will have a look.

@IshaanDesai IshaanDesai changed the title Remove the dictionary values scalar and vector for data name keys in Micro Manager config files Several updates to the Micro Manager config files Jan 27, 2025
@IshaanDesai IshaanDesai requested a review from MakisH January 27, 2025 10:46
Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

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

The changes make sense to me.

@MakisH Should we document the version dependency somewhere? (We now require the newest develop branch of the Micro Manager)

As also stated in precice/micro-manager#133, these changes are breaking for the micro manager, but it is still in the v0.x cycle, so this is totally fine.

I think that requiring the newest develop is fine in this case, because it is experimental software. However, this will lead to unreproducible results, and if we add this case to the system tests, for example, they might often fail because of the different version. The usual tradeoff.

What we should remember would be to fix the version in before we do a Distribution release. I opened an issue for discussing the how: #610

I think that a statement in the README of the case that explains the following would be helpful:

  • The case automatically gets the latest version, which might trigger issues if it is newer if it is newer than the tutorials version.
  • Which set of versions this is known to work with and how to set them.

@IshaanDesai
Copy link
Member Author

Holding off on merging this until a release of the Micro Manager is done. As the Micro Manager is still in initial development, releases are done rather flexibly and frequently. Hence it makes sense to release the Micro Manager and point to a release in the requirements.txt of the participants which use it.

@uekerman
Copy link
Member

Both options, pointing to develop or new release, are OK for me. Depends probably on the frequency of such changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants