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

RemoteBCL does not enforce min_compatible for measures #5230

Open
jmarrec opened this issue Jul 15, 2024 · 6 comments
Open

RemoteBCL does not enforce min_compatible for measures #5230

jmarrec opened this issue Jul 15, 2024 · 6 comments

Comments

@jmarrec
Copy link
Collaborator

jmarrec commented Jul 15, 2024

Issue overview

Measures with min_compatible > current CLI version should be be gotten from BCL

Current Behavior

The release of 3.8.0 broke a lot of measures, most notably the openstudio_results measure, because the openstudio-extension-gem's ruby methods were removed from the CLI, and replaced with equivalent C++ methods, but in a non-backward compatible way.

As a result, these measures were adjusted and recently released with new versions that have "min_compatible" = 3.8.0.

RemoteBCL does not check that one bit, neither in searchComponentLibrary + getMeasure nor in measuresWithUpdates.

This is problematic because it means every user of the CLI < 3.8.0 would get the newest version which is not compatible with their current CLI.

NREL/openstudio-common-measures-gem#169 (comment)

Expected Behavior

RemoteBCL should check min_compatible/max_compatible.

I would expect that:

  • A measure with min_compatible > current cli version should NOT be in the results
  • A measure with max_compatible < current cli version should NOT be in the results
  • The newest compatible version of a measure should be in the results
    • If a measure has a reivision1 that is compatible with the current CLI, but the revision2 (latest) isn't, revision1 should be suggested

Steps to Reproduce

test_bcl.rb

remoteBCL = OpenStudio::RemoteBCL.new
openstudio_results_uid = "a25386cd-60e4-46bc-8b11-c755f379d916"
responses = remoteBCL.searchMeasureLibrary(openstudio_results_uid, 980)
raise "More than 1 responses" unless responses.size == 1
response = responses.first
if response.respond_to?(:versionModified)
  puts "#{response.name} - versionId=#{response.versionId}, versionModified=#{response.versionModified.get.to_s}"
else
  puts "#{response.name} - versionId=#{response.versionId}"
end
$ /usr/local/openstudio-3.8.0/bin/openstudio test_bcl.rb
Openstudio results - versionId=5b4d387a-f537-4693-a9eb-6192957b6c1b, versionModified=2024-Jul-11 14:31:57
$ /usr/local/openstudio-3.7.0/bin/openstudio test_bcl.rb
Openstudio results - versionId=5b4d387a-f537-4693-a9eb-6192957b6c1b

Possible Solution

Update RemoteBCL code, and expand BCLSearchResult to include min/max compatible. Might need a server change on the BCL side itself.

Details

Environment

Some additional details about your environment for this issue (if relevant):

  • Platform (Operating system, version): all
  • Version of OpenStudio (if using an intermediate build, include SHA): 3.8.0

Context

Found because the OpenStudio Application 1.7.1 which uses OS SDK 3.7.0 has this issue since July 11.

@jmarrec
Copy link
Collaborator Author

jmarrec commented Jul 15, 2024

I don't think there's any change need on the BCL server itself.

import requests

r = requests.get('https://bcl.nrel.gov/api/search/?fq=uuid:a25386cd-60e4-46bc-8b11-c755f379d916&all_content_versions=1')
r.raise_for_status()
data = r.json()

for i, result in enumerate(data['result']):
    m = result['measure']
    assert m['name'] == 'openstudio_results'
    assert m['uuid'] == 'a25386cd-60e4-46bc-8b11-c755f379d916'
    print(f"{i} - openstudio_min_compatible={m['openstudio_min_compatible']}")
0 - openstudio_min_compatible=3.8.0
1 - openstudio_min_compatible=3.1.0
2 - openstudio_min_compatible=3.1.0
3 - openstudio_min_compatible=3.1.0
4 - openstudio_min_compatible=3.1.0
5 - openstudio_min_compatible=3.1.0
6 - openstudio_min_compatible=3.1.0

@jmarrec
Copy link
Collaborator Author

jmarrec commented Jul 15, 2024

@jmarrec
Copy link
Collaborator Author

jmarrec commented Jul 15, 2024

I think the best location to plug into is

BCLSearchResult searchResult(componentElement);

openstudio_min_compatible is a top level key that's provided by the BCL api. Is there an openstudio_max_compatible to match when needed (when the measure.xml has max_compatible)? @kflemin could you please help here? I couldn't find the repo/location where openstudio_min_compatible is created, perhaps that is private.

It'd be faster do this, before trying to instantiate a BCLSearchResult which will parse all files, and set the min/Max compatible and then looping on all of these to ensure all files are compatible.

@jmarrec
Copy link
Collaborator Author

jmarrec commented Jul 15, 2024

Maybe I spoke too fast, I think I found the right schema, and it does have 'openstudio_max_compatible': https://github.com/NREL/new-bcl/blob/2d2e742a75086683fab954101f069e9813b3ea7d/bcl/static/assets/json/component_schema.json#L46-L53

@jmarrec
Copy link
Collaborator Author

jmarrec commented Jul 15, 2024

Right now our RemoteBCL only gets one result, which I suppose is faster.

So I thought about whether we could just change the way we query the BCL to have the filtering done on the bcl server, so we do not have to do all_content_versions=1

You can filter by min_compatible when searching the BCL

https://bcl.nrel.gov/api/search/?fq=uuid:a25386cd-60e4-46bc-8b11-c755f379d916&all_content_versions=1&fq=openstudio_min_compatible:3.1.0

But it's not helpful; it does not seem to support equality operators, just "="

Ideally you'd need to be able to do something like &fq=openstudio_min_compatible:<=3.7.0 (where 3.7.0 is the current CLI version)

@DavidGoldwasser
Copy link
Collaborator

@kflemin wanted to let you know about this. We can talk about 3.9 for this.

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

No branches or pull requests

3 participants