Increase test timeout

Increase the test timeout for directory monitor tests as we are seeing this fail fairly often with the ARM64 builds (see, e.g.).

Codecov Report

Merging #3261 (471a0b0) into main (0daa84d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #3261   +/-   ##
=======================================
  Coverage   29.65%   29.65%
=======================================
  Files         332      332
  Lines       39290    39290
=======================================
  Hits        11651    11651
  Misses      27639    27639

Impacted Files
Coverage Δ

src/agent/onefuzz/src/monitor/tests.rs
100.00% <ø> (ø)


google-cloud-logging should allow user to set all fields in the LogEntry

Currently the interface of com.google.cloud.logging.LogEntry does not let me specify the sourceLocation among other fields. It would be very nice to be able to set them.

FYI @lesv @garrettjonesgoogle Currently sourceLocation field is not supported by the client.

@jabubake FYI


AssetBundle corrupting LethalExpansion

Hello, I was able to generate my assetbundle- but now when I load it into the game and start a server the entire LethalExpansion mod gets disabled. No new moons, no extra scrap items, etc. I am sure that it is my additional scrap item mod that is breaking LethalExpansion yet I don't know why as the only difference I see is the lack of the printout in the command prompt when creating the items and adding the moons.

^^ This is what it looks like with my personal trophy.lem assetbundle.

^^ This is what it looks like without my trophy.lem assetbundle.
I'll include the files below. Thanks in advance- I'm new to this and trying to get the hang of it.
TrophyMod.zip

Hello @GoblinFiend
I will take a look today

@HolographicWings
Thanks <3

@HolographicWings Thanks <3

Can you send me the lem file too ?

@HolographicWings
Thanks so much for your detailed response I'm working on some of these issues right now. I don't know where to find the scan node and apply it to my new item but other than that I've addressed all the other issues as of now. If possible a screenshot of the ScanNode element in the prefab would be great if possible!

@HolographicWings Thanks so much for your detailed response I'm working on some of these issues right now. I don't know where to find the scan node and apply it to my new item but other than that I've addressed all the other issues as of now. If possible a screenshot of the ScanNode element in the prefab would be great if possible!

Nevermind, I found it. New to unity :p

@HolographicWings
I just wanted to thank you for all the assistance once again. Everything now works and I am super happy with the result! It looks really good and works just as intended. , I have two more questions / concerns though. When holding the item it is sideways as opposed to right side up. How do I change this?
Also, I would like to know more about the "weight" in the spawn chance. Can I weight it at .5? Is 100 the max weight? Some more clarity about how this system works would be great!
When looking at the item in the dark it also looks very dark almost black, which I imagine is because of the reflectivity of it. Do I need a normal map or something like: "Assets/Mods/TemplateMod/Items/Scraps/tape/survivaltools2_Tape_MetallicSmoothness.png"
To help remedy this?
You're amazing :D

@HolographicWings I just wanted to thank you for all the assistance once again. Everything now works and I am super happy with the result! It looks really good and works just as intended. I have some more questions / concerns though. When holding the item it is sideways as opposed to right side up. How do I change this?
Also, I would like to know more about the "weight" in the spawn chance. Can I weight it at .5? Is 100 the max weight? Some more clarity about how this system works would be great!
Other things: How do I find the name of the SFX? would it be DropBigBolt? When looking at the item in the dark it also looks very dark almost black, which I imagine is because of the reflectivity of it. Do I need a normal map or something like: "Assets/Mods/TemplateMod/Items/Scraps/tape/survivaltools2_Tape_MetallicSmoothness.png" To help remedy this?
You're amazing :D

It's the Offsets in the scrap
Theres a scene at Assets/Mods/LethalExpansion/Scenes/ItemPlaceTest.unity you can use it to place your scrap either on ItemHoldAxis or ItemGroundAxis, theses are references to rotate and scale your items corectly before set the offsets


Allow specifying the precision?

Any plan or desire to allow specifying the precision with 2 being the default? Seems useful to account for regulatory requirements or even just personal preference. I'm imagining something like this:
Dinero({ amount: 1000, currency: 'USD' })
  .toFormat('$0,0.00'); // '$10.00'
Dinero({ amount: 10000000, precision: 6, currency: 'USD' })
  .toFormat('$0,0.00'); // '$10.00'

I'd be happy to PR this!

There's a project for this. You can click "Menu" to see the description. I'm assigning it to this issue.
I feel like this deserves a discussion before starting to code 😊

Awesome. Do you have an idea about how to have this discussion? Just talk about it on this issue or set up a call?
I can throw together a throw-away PR that has the most naivë implementation I can think of as a starting point for a discussion if that helps materialize some ideas and concerns.

There are two approaches for adding the concept of precision: added to, or baked in the exponent.
For the record, the exponent is the factor by which you multiply 10 to get the number of sub-units in a unit. For dollars, the exponent is 2 as 10^2 = 100.
Dinero.js doesn't support exponents properly yet, but will in v2. Now even though it doesn't currently have the concept of an exponent, it implicitly does by dividing by 100. This implicit behavior was reflected in v1.1.2 to prepare for future support.
The exponent defacto indicates the default number of possible decimal places for a billable amount. Yet you may want to represent prices with more precision:

unit price for items sold in higher quantity, like stuff in bulk or gas
avoid rounding during calculations to preserve precision

So this concept, loosely named precision here, aims at "adding" support for these fractional units so they can be represented and calculated.
Back to the two approaches, here how you can think about it:
Solution 1: Precision is added to the exponent
In this case, precision starts at 0. Anything more adds up to the existing exponent of the currency.
Whatever the exponent is, precision adds up decimal places.
Dinero({ amount: 104545, currency 'USD' })
  .toUnit() // 1045.45

Dinero({ amount: 104545, currency 'USD', precision: 2 })
  .toUnit() // 10.4545, or 104545 / 10^2+2

Solution 2: Precision is "baked in" the exponent
In this case, precision starts at the existing exponent of the currency. Giving a precision that's equal to the exponent is like not giving a precision at all. Giving less throws an error, and giving more adds starting from 0.
Dinero({ amount: 104545, currency 'USD' })
  .toUnit() // 1045.45

Dinero({ amount: 104545, currency 'USD', precision: 2 })
  .toUnit() // 1045.45, or 104545 / 10^2+2-2

Both solutions have advantages and flaws. I'm leaning towards solution 2, here's why.
I think it's easier to understand for the end user, and more straight-forward. When you see precision: 2, you expect two decimal places. If you have to always handle the same amount of decimal places even regardless of the currency, you can do it easily.
The only "flaw" I can think of is that adding a precision that's equal to the existing component does nothing, but in the end, if you have to use precision that's because you need more than what exponent already gives you.
I think solution 1 is confusing.
Dinero({
  amount: 104541,
  currency 'USD',
  precision: 4
})

If you see the above piece of code, you have no idea what the end price is, and it gets worse as you need more decimal places. It's definitely not straight-forward that the above object represents $0.104541. If you're new on a project, you will need some help figuring out the underlying mechanism. If you need to build a new object that represents an amount with support for more decimal places than it does by default, you'll need to do some mental calculations that can become annoying and cumbersome in the long run.
I'm 100% open to debate, don't hesitate to jump in if you have converging or diverging opinions 👍

Thanks @sarahdayan for such a thorough overview of the two solutions. I believe I have a good case for preferring Solution 1, so I'll add it here to further the debate.
There are only a few possibilities I can think of for needing addtional precision beyond the sub-unit of the currency.

Expressing a monetary value for something that is usually in bulk, like flour, gasoline, or a financial instrument like a future. You've already pointed out this use case.
Regulatory requirements. You might be required by some regulatory body to calculate to a certain fraction of the sub-unit.
You have a non-regulatory business requirement that requires or suggests extra precision when doing calculations.
You anticipate converting between currencies and need extra precision.

I'm sure there are a few others, but my guess (based on a survey of other implementations) is that this feature is not widely needed or used.
Given those use cases, my feeling is that the precision that you really care about is additional precision beyond the sub-unit. As you pointed out already, Solution 2 has a case where you can specify a value and it's the same as not specifying a value. That's not a big problem. I see throwing a runtime exception for specifying a smaller precision than the exponent as more problematic, personally.
If we can agree that it is not useful to set the precision to the value of the exponent, than the only meaningful values are positive integers greater than the exponent. Furthermore, all four cases suggest to me that the added precision is a fixed value and does not depend on the particular currency. Certainly that is just my impression of the problem-space and not a requirement. If we believe in my assumption about fixed-precision, I believe it suggests Solution 1.
On to the objection, which is totally valid, that the user will not be able to convert from the Dinero.amount back to the actual monetary unit in their heads: I don't think people will be dealing with the raw .amount property much directly.
It is very likely that in an actual system you will not be entering in .amount values directly from a provided variable or a Number literal. That assumes that the storage of these values is anticipating the exponent already, so likely you'll have some sort of function that converts from whatever your initial representation is to the properly padded .amount value. Something like:
function fromDatabaseNumberToAmount(dbNum, exponent) {
  return dbNum * Math.pow(10, exponent);
}

If you're use case requires extra precision, you can easily just add your precision in this function:
function fromDatabaseNumberToAmount(dbNum, exponent, precision) {
  return dbNum * Math.pow(10, exponent + precision);
}

Where does the exponent come from? That either needs to be communicated by the source of the amount in the first place (database, server, in-memory value) or we need to provide a way to query for the exponent.
Another idea is to provide helper functions to convert from format strings or the unit numbers back into Dinero:
const item = db.getById('itemId'); // { price: '1,000.45' }
const dString = Dinero.fromString(item.price, '0,0.0', { currency: 'USD', precision: 4 });
dString.getAmount(); //<PHONE_NUMBER>
dString.getUnit(); // 1000.45

const item = db.getById('itemId'); // { price: 1000.45 }
const dUnit = Dinero.fromUnit(item.price, { currency: 'USD', precision: 4 });
dUnit.getAmount(); //<PHONE_NUMBER>
dUnit.getUnit(); // 1000.45

This takes the ambiguity out of it, and allows us to keep the exponent internal to the library. In Solution 2, you have to know the exponent in order to set the additional precision and not get a runtime exception. I believe the only time you need to reason about the .amount literal is during debugging, and it's easy enough to call #toUnit on the instance to take the guess work out of it. FWIW, you have to know the exponent when reasoning about the .amount, too.

I definitely can sympathize with the idea that the name precision makes it seem like it should include the subunit (like USD is precision 2, JPY is precision 0), so I think we need to maybe consider another name if we go with Solution 1.

Thanks a lot for your input @scotttrinh. Here are my thoughts about it.
I agree that precision isn't an ideal name. It's ambiguous and no matter what solution is picked, it needs to change accordingly. In solution 1, it's more a concept of additionalSubUnitFractions, while in solution 2, this would be subUnitFractions. I'm not settled on these terms but let's use them for the sake of clarity in the debate.

If we can agree that it is not useful to set the precision to the value of the exponent, then the only meaningful values are positive integers greater than the exponent.

I agree with this, but I feel like how it translates in terms of code isn't satisfying because it creates serious cognitive overhead.

All four cases suggest to me that the added precision is a fixed value and does not depend on the particular currency.

The advantage of additional sub-unit fractions is that you can always add the same amount of additional decimal places, regardless of the currency. It's always "plus x". The inconvenience is that, if you want a specific number of overall decimal places, you need to do some mental math.
Solution 1
I want 4 decimal places, I need to know what is the exponent and do exponent - 4 to know additionalSubUnitFractions.
// For an amount of 1
Dinero({ amount: 10000, currency: 'USD', additionalSubUnitFractions: 2 })
Dinero({ amount: 100000, currency: 'IQD', additionalSubUnitFractions: 1 })

Solution 2
I want 4 decimal places, I can simply indicate it.
// For an amount of 1 dollar/dinar
Dinero({ amount: 10000, currency: 'USD', subUnitFractions: 4 })
Dinero({ amount: 100000, currency: 'IQD', subUnitFractions: 4 })

The issue of fluctuating exponents
Currently, exponent equals to 2, all the time. It means that if you want to use the concept of precision to circumvent it, this is how you'd do it right now:
Solution 1
// For an amount of 1 dinar
Dinero({ amount: 1000, currency: 'IQD', additionalSubUnitFractions: 1 })

Solution 2
// For an amount of 1 dinar
Dinero({ amount: 1000, currency: 'IQD', subUnitFractions: 3 })

As exponent support is added, Dinero objects created with solution 2 will keep on working. All you'll have to do is remove the unnecessary subUnitFractions parameter, but it won't matter if you don't. With solution 1, you'll need to do a major migration. In the example above, your Dinero object representing 1 dinar will suddenly represent 0.1 dinar.
I know that all upgrades to a new major version involves migrating your app, but this one would definitely be annoying.
In addition to that, we can't guarantee that exponent won't change. If Iraqi dinar, that currently still officially use fils, but that inflation has made obsolete, finally drops sub-unit support (like Japanese Yens), exponent will shift from 3 to 0. With solution 1, this will mess up all existing implementations.
The thing is, we shouldn't deprecate the API every time the currency dataset is changed, just because the implementation might suffer. With solution 1, this will be inevitable. With solution 2, instead of throwing when the precision is lower than the exponent (for example, if exponent goes from 0 to 3 and you had specified a precision of 2), you can just force the precision to be at least as much as the exponent, regardless of what the user specified. An additional warning could be thrown in dev mode but nothing would break.
I believe solution 2 isn't only clearer, it's also more robust and future-proof.

Also consider large precision numbers for virtual currencies. Bitcoin has a precision of 8 decimals and Ethereum has a precision of 18 decimals. Is there support for 256 bit numbers?

@joelnet Precision and exponent are different concepts. Bitcoins have an exponent of 8, while the issue here is allowing representation for "split sub-units", values that are virtually not billable (like $250.525) but may be needed for more precise calculations and or formatting purposes.
Bitcoins aren't ISO 4217, so they wouldn't be automatically supported as soon as #9 is taken care of. Yet, you'll be able to use precision to "bypass" that and create Dinero objects with bitcoins anyway by doing Dinero({ amount:<PHONE_NUMBER>, currency: 'XBT', precision: 8 }) 👍

Thanks for the explanation. I have run into issues with precision before, I guess I never fully understood it until your explanation. I ended up solving it by increasing the exponent and then rounding for my final result.
Now that I understand the difference between precision and exponents, I need to go back in time and fix my code. Why can't I ever find my time machine when I need it most?
Cheers!

@sarahdayan I definitely think backwards-compatibility is something to think about and would like to avoid breaking changes as much as possible! I think the basis of our disagreement is on what is the normal case:

fixed number of additional decimal places
fixed number of decimal places

The only evidence I have to support thinking Use 1 is the more common one is the idea of supporting fractional prices on bulk goods. You wouldn't want to encode the decimal places, since the extra precision might be truncated in the case of exponents greater than your usual. For example, you have an inventory application that allows subunit precision for bulk unit pricing, what would you put as subUnitFractions where it would be compatible with any exponent? Without being able to lookup the exponent you simply could not allow specifying additional precision.
For your example where the exponent changes to go beyond the specified precision (0 to 3), it would still break anyone who has serialized the .value instead of the #toUnit() value. Example:
// JPY exponent: 0
const d = new Dinero({
  value: 1000,
  currency: 'JPY',
  subUnitFractions: 0
});
persistToStorage('price', d); // { value: 1000, currency: 'JPY', subUnitFractions: 0 }
d.toUnit(); // 1000

// Sometime in the future when the yen gets very valuable
// JPY exponent: 3
const dSpec = fromStorage('price'); // { value: 1000, currency: 'JPY', subUnitFractions: 0 }
const d = new Dinero(dSpec);
d.toUnit(); // 1

I still maintain that no one will be putting in non-"converted" values when creating a Dinero object. But, I would imagine that you would serialize the configuration object and reuse it and expect the same value, which is not the case in Solution 2.
FWIW, I think exponents changing is, unavoidably, a breaking change. But, in a sense, it's a breaking change for the entire financial system. We should try to consider how to best serve the users, but I think we can do that by providing helper functions to create the correct .value property from a string or a unit, and encourage the users to store the #toUnit or #toFormat value instead of anything that relies on the .value value.

So, there's been quite some debate 🙂 I think at this point it's time to go back to the initial problem and simplify, so we can move on.
Currently, the notion of decimal places is hardcoded to 2. It keeps people from accurately representing currencies that use less or more (e.g.: IQD, JPY), and it keeps people from representing monetary values with more precision than the currency allows by default (e.g. $10.4545).
We want to give more flexibility. In the end, people need to be able to indicate, for the amount they specify, what is the number of sub-units it represents (or if there are none). Whether you want to say precision: 0 because you're using Yens, or because you want to manipulate mains units only, that's your business.
So, there should be one concept. Let's call it precision for now. This is the number of sub-units you want to represent. When you call toUnit, what gets returned is amount / 10^precision. It's simple, easy to understand, fits most cases.
Later, when a dataset is added with all the default exponents for each ISO 4217 currency, the default 2 will be replaced by the corresponding exponent to the currency you gave. So if you didn't specify a precision, this will be the new default value. If you did specify a precision, this overrides it. This is of course a breaking change, which is why it will only happen in v2.
But the bottom line is, you have the possibility to specify how many sub-units you're representing. Then it's all about what the default value is.
For more specific use cases, for example like the @scotttrinh gave where you might want to specify additional decimal places to the default ones depending on the currency, this can be handled by a converting method that adds or removes precision and changes the amount accordingly:
const d = Dinero({ amount: 100, currency: 'EUR' }) // precision isn't specified, so by default it's 2

d.addPrecision(3) // precision shifts to 5, amount shifts to 100000
d.removePrecision(4) // precision shifts to 1, amount shifts to 10

d.setPrecision(3) // precision is set to 3, amount remains untouched

This solution offers simplicity, reduces the risks for bugs, and allows to handle a lot of use cases.

@sarahdayan
I like the idea of having those precision shifting functions! To be clear, they wouldn't mutate, they'd return new Dinero objects, right? This definitely accomplishes the use-case I had in mind at the expense of creating the intermediate object, but that's a really tiny price to pay. Sounds like a good way forward!
I'll open a separate issue for the possible enhancement of adding deserializing (#fromUnit or #fromFormat) helper functions that might also include some of this functionality and is compatible with the idea of precision always indicating the decimal precision of the unit. I agree that is the most clear way to represent this.

@scotttrinh Yes of course, always new objects. The library is and must remain immutable 🙂

:tada: This issue has been resolved in version 1.3.0 :tada:
The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


DataFetcherExceptionResolverAdapter doesn't get called when using a custom QueryExecutionStrategy

I've found a bug that occurs when using DataFetcherExceptionResolverAdapter and a custom QueryExecutionStrategy.
For some reason, if you return a failed future the exception just gets bubbled up without passing through DataFetcherExceptionResolverAdapter.
I've got a reproducible example here: https://github.com/JamesPeters98/SpringGraphQLErrorTest/
There is a Query and Mutation example. The mutation gets resolved correctly:

But not the query:

If you remove the custom QueryExecutionStrategy the exception gets resolved

Looking into this more, it appears to be because the custom QueryExecutionStrategy is missing the defaultDataFetcherExceptionHandler created in AbstractGraphQlSourceBuilder by Spring, but it doesn't seem that there's any way to access this from GraphQL Java.
I'm not sure what the best solution to this is? Maybe there should be a default Bean for DataFetcherExceptionHandler? Or add Getters for the exceptionResolvers field so we can create our own instance of ExceptionResolversExceptionHandler

Thanks for the sample. Yes, I see the challenge there. To create an ExecutionStrategy, you need a DataFetcherExceptionHandler, but our implementation is private and it's created inside GraphQlSource.Builder which accepts a list of DataFetcherExceptionResolver types.
We could expose a static factory method on DataFetcherExceptionResolver to create a DataFetcherExceptionHandler from a list of resolvers. Then you would inject ObjectProvider<DataFetcherExceptionResolver> into your bean method and use that to create the DataFetcherExceptionHandler to pass to the custom ExecutionStrategy.


iris==3.11.0rc0 causing some issues

we briefly picked up the new RC0 of iris==3.11 in the nightly tests that caused some issues with what appears to be deprecated/removed functionality, but then the GA tests from the most recent merge reverted the iris version to 3.10 https://github.com/ESMValGroup/ESMValTool/actions/runs/11592095426 - @trexfeathers is your RC0 stable enough, or we have to pin to <3.11 for now before you guys iron out all the teething issues? :beer:

Hi, this should now be stable!
I'd messed up a file name and released the rc on main, this is now marked as broken in conda-forge, and has been taken off the channel. Apologies for the oversight there.
You should now only get 3.10 unless you specify the 3.11rc0 as shown in the release page.

@ESadek-MO many thanks! I'll keep our environment clean (ie no hard pin with ==3.11.0rc0) for now, and will wait for the stable 3.11.0 to make its way. BTW conda forge still have issues but only with PRs opened during their Monday downtime, new PRs run builds OK :beer:


cupy.array raises TypeError for list of arrays of different dtypes

numpy.array seems to be called unexpectedly in cupy.array for such inputs.
>>> numpy.array([cupy.zeros((2, 3), dtype='f'), cupy.zeros((2, 3), dtype='d')])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "cupy/core/core.pyx", line 1171, in cupy.core.core.ndarray.__array__
    raise TypeError(
TypeError: Implicit conversion to a NumPy array is not allowed. Please use `.get()` to construct a NumPy array explicitly.

Thank you Imanishi-san for making this Issue.
Shouldn't the example use cupy.array function instead of numpy.array function?
cupy.array([cupy.zeros((2, 3), dtype='f'), cupy.zeros((2, 3), dtype='d')])
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-5-2b780e71a55b> in <module>
----> 1 cupy.array([cupy.zeros((2, 3), dtype='f'), cupy.zeros((2, 3), dtype='d')])

~/research/dev-cupy/cupy/cupy/creation/from_data.py in array(obj, dtype, copy, order, subok, ndmin)
     41
     42     """
---> 43     return core.array(obj, dtype, copy, order, subok, ndmin)
     44
     45

~/research/dev-cupy/cupy/cupy/core/core.pyx in cupy.core.core.array()
   1983 # -----------------------------------------------------------------------------
   1984
-> 1985 cpdef ndarray array(obj, dtype=None, bint copy=True, order='K',
   1986                     bint subok=False, Py_ssize_t ndmin=0):
   1987     # TODO(beam2d): Support subok options

~/research/dev-cupy/cupy/cupy/core/core.pyx in cupy.core.core.array()
   2062     # fallback to numpy array and send it to GPU
   2063     # Note: dtype might not be numpy.dtype in this place
-> 2064     return _send_object_to_gpu(obj, dtype, order, ndmin)
   2065
   2066

~/research/dev-cupy/cupy/cupy/core/core.pyx in cupy.core.core._send_object_to_gpu()
   2129         else:
   2130             order = 'C'
-> 2131     a_cpu = numpy.array(obj, dtype=dtype, copy=False, order=order,
   2132                         ndmin=ndmin)
   2133     a_dtype = a_cpu.dtype  # converted to numpy.dtype

~/research/dev-cupy/cupy/cupy/core/core.pyx in cupy.core.core.ndarray.__array__()
   1169         # configure flag that allows implicit conversions to NumPy array.
   1170         # (See https://github.com/cupy/cupy/issues/589 for the detail.)
-> 1171         raise TypeError(
   1172             'Implicit conversion to a NumPy array is not allowed. '
   1173             'Please use `.get()` to construct a NumPy array explicitly.')

TypeError: Implicit conversion to a NumPy array is not allowed. Please use `.get()` to construct a NumPy array explicitly.

Sorry, I fixed the description. Raising TypeError for implicit device-to-host conversion: numpy.array(cupy_arrays) is intended behavior.


Update the Sub-GHz Remote app for the SDK version 0.105 f7

Hello!
Your Sub-GHz Remote app version 1.4 fails to build with the SDK version 0.105.0-rc f7. Please update the app in your repository and create a new pull request to the catalog repository with the updated manifest file.

IMPORTANT: Don’t forget to update the app version and commit version in the manifest file.

View logs
View build actions
Flipper Team

Hello sir,
I just updated my flipper and absolutely adore your application - would you mind updating it, or telling me if there's a way I can contribute to maintaining this application? Why does flipper require you to update the app everytime a new firmware is out?

I am again here!
Your Sub-GHz Remote app version 1.4 fails to build with the SDK version 1.0.0-rc f7. Please update the app in your repository and create a new pull request to the catalog repository with the updated manifest file.

IMPORTANT: Don’t forget to update the app version and commit version in the manifest file.

View logs
View build actions
Flipper Team

Hello sir,
I just updated my flipper and absolutely adore your application - would you mind updating it, or telling me if there's a way I can contribute to maintaining this application? Why does flipper require you to update the app everytime a new firmware is out?

All is updated


Explainability with hcs

output comments: for ip-block as src or dst - no need to add policy configurations or resources configurations.

Done - "ipblocks had no configurations" explanation now replace the explanation for Ipblocks.

Test run issue: when running with explain: ALL and without -opt=true , nca crashes in explain_all function, though I would expect this function should not be called.

I could not re-create the crash. Perhaps was tested on an older version? the current version protects this case.


[SPARK-6775] [SPARK-6776] [SQL] [WIP] Refactors converters for converting Parquet records to row objects

This PR simplifies the CatalystConverter class hierarchy, and implements backwards-compatibility rules for them.  The new class hierarchy basically resembles AvroIndexedRecordConverter in parquet-avro.
This is still in WIP status. I'm opening this PR to check whether it passes Jenkins.
TODO:

[ ] Pass all tests.
[ ] Tests for legacy Parquet data format.

Major changes:

Replaces CatalystConverter trait with a much simpler ParentContainerUpdater.
Now instead of extending the original CatalystConverter trait, every converter class accepts an updater whose's responsible to set the converted value to some parent container. For example, appending array elements to a parent array buffer, appending a key-value pairs to a parent mutable map, or setting a converted value to some specific field of a parent row.
This simplifies the design since converters don't need to care about details of their parent converters anymore.

Replaces CatalystRootConverter, CatalystGroupConverter and CatalystPrimitiveRowConverter with CatalystRowConverter
Specifically, now all row objects are represented with SpecificMutableRow during conversion.

Removes CatalystArrayContainsNullConverter and CatalystNativeArrayConverter, revamped CatalystArrayConverter
CatalystNativeArrayConverter should be designed with the intention of avoiding boxing costs. However, the way it uses Scala generics actually doesn't achieve this goal.
The new CatalystArrayConverter handles both nullable and non-nullable array elements.

Implemented backwards-compatibility rules in CatalystArrayConverter
When Parquet records are being converted, schema of Parquet files should have already been verified. So we only need to care about the structure rather than field names in the Parquet schema. Since all map objects represented in legacy systems have the same structure as the standard one (see [backwards-compatibility rules for MAP] 1), we only need to deal with LIST (namely array) here.

Test build #29858 has started for   PR 5422 at commit 0de2e8b.

Test build #29859 has started for   PR 5422 at commit 9522949.

Test build #29858 has finished for   PR 5422 at commit 0de2e8b.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29858/
Test FAILed.

Test build #29859 has finished for   PR 5422 at commit 9522949.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29859/
Test FAILed.

Test build #29868 has started for   PR 5422 at commit 02d78c4.

Test build #29869 has started for   PR 5422 at commit 2529b76.

Test build #29868 has finished for   PR 5422 at commit 02d78c4.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29868/
Test FAILed.

Test build #29869 has finished for   PR 5422 at commit 2529b76.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29869/
Test FAILed.

Test build #31021 has finished for   PR 5422 at commit 2529b76.

This patch fails Spark unit tests.
This patch does not merge cleanly.
This patch adds no public classes.
This patch adds the following new dependencies:

RoaringBitmap-0.4.5.jar
activation-1.1.jar
akka-actor_2.10-2.3.4-spark.jar
akka-remote_2.10-2.3.4-spark.jar
akka-slf4j_2.10-2.3.4-spark.jar
aopalliance-1.0.jar
arpack_combined_all-0.1.jar
avro-1.7.7.jar
breeze-macros_2.10-0.11.2.jar
breeze_2.10-0.11.2.jar
chill-java-0.5.0.jar
chill_2.10-0.5.0.jar
commons-beanutils-1.7.0.jar
commons-beanutils-core-1.8.0.jar
commons-cli-1.2.jar
commons-codec-1.10.jar
commons-collections-3.2.1.jar
commons-compress-1.4.1.jar
commons-configuration-1.6.jar
commons-digester-1.8.jar
commons-httpclient-3.1.jar
commons-io-2.1.jar
commons-lang-2.5.jar
commons-lang3-3.3.2.jar
commons-math-2.1.jar
commons-math3-3.1.1.jar
commons-net-2.2.jar
compress-lzf-1.0.0.jar
config-1.2.1.jar
core-1.1.2.jar
curator-client-2.4.0.jar
curator-framework-2.4.0.jar
curator-recipes-2.4.0.jar
gmbal-api-only-3.0.0-b023.jar
grizzly-framework-2.1.2.jar
grizzly-http-2.1.2.jar
grizzly-http-server-2.1.2.jar
grizzly-http-servlet-2.1.2.jar
grizzly-rcm-2.1.2.jar
groovy-all-2.3.7.jar
guava-14.0.1.jar
guice-3.0.jar
hadoop-annotations-2.2.0.jar
hadoop-auth-2.2.0.jar
hadoop-client-2.2.0.jar
hadoop-common-2.2.0.jar
hadoop-hdfs-2.2.0.jar
hadoop-mapreduce-client-app-2.2.0.jar
hadoop-mapreduce-client-common-2.2.0.jar
hadoop-mapreduce-client-core-2.2.0.jar
hadoop-mapreduce-client-jobclient-2.2.0.jar
hadoop-mapreduce-client-shuffle-2.2.0.jar
hadoop-yarn-api-2.2.0.jar
hadoop-yarn-client-2.2.0.jar
hadoop-yarn-common-2.2.0.jar
hadoop-yarn-server-common-2.2.0.jar
ivy-2.4.0.jar
jackson-annotations-2.4.0.jar
jackson-core-2.4.4.jar
jackson-core-asl-1.8.8.jar
jackson-databind-2.4.4.jar
jackson-jaxrs-1.8.8.jar
jackson-mapper-asl-1.8.8.jar
jackson-module-scala_2.10-2.4.4.jar
jackson-xc-1.8.8.jar
jansi-1.4.jar
javax.inject-1.jar
javax.servlet-3.0.0.v201112011016.jar
javax.servlet-3.1.jar
javax.servlet-api-3.0.1.jar
jaxb-api-2.2.2.jar
jaxb-impl-2.2.3-1.jar
jcl-over-slf4j-1.7.10.jar
jersey-client-1.9.jar
jersey-core-1.9.jar
jersey-grizzly2-1.9.jar
jersey-guice-1.9.jar
jersey-json-1.9.jar
jersey-server-1.9.jar
jersey-test-framework-core-1.9.jar
jersey-test-framework-grizzly2-1.9.jar
jets3t-0.7.1.jar
jettison-1.1.jar
jetty-util-6.1.26.jar
jline-0.9.94.jar
jline-2.10.4.jar
jodd-core-3.6.3.jar
json4s-ast_2.10-3.2.10.jar
json4s-core_2.10-3.2.10.jar
json4s-jackson_2.10-3.2.10.jar
jsr305-1.3.9.jar
jtransforms-2.4.0.jar
jul-to-slf4j-1.7.10.jar
kryo-2.21.jar
log4j-1.2.17.jar
lz4-1.2.0.jar
management-api-3.0.0-b012.jar
mesos-0.21.0-shaded-protobuf.jar
metrics-core-3.1.0.jar
metrics-graphite-3.1.0.jar
metrics-json-3.1.0.jar
metrics-jvm-3.1.0.jar
minlog-1.2.jar
netty-3.8.0.Final.jar
netty-all-4.0.23.Final.jar
objenesis-1.2.jar
opencsv-2.3.jar
oro-2.0.8.jar
paranamer-2.6.jar
parquet-column-1.6.0rc3.jar
parquet-common-1.6.0rc3.jar
parquet-encoding-1.6.0rc3.jar
parquet-format-2.2.0-rc1.jar
parquet-generator-1.6.0rc3.jar
parquet-hadoop-1.6.0rc3.jar
parquet-jackson-1.6.0rc3.jar
protobuf-java-2.4.1.jar
protobuf-java-2.5.0-spark.jar
py4j-<IP_ADDRESS>.jar
pyrolite-2.0.1.jar
quasiquotes_2.10-2.0.1.jar
reflectasm-1.07-shaded.jar
scala-compiler-2.10.4.jar
scala-library-2.10.4.jar
scala-reflect-2.10.4.jar
scalap-2.10.4.jar
scalatest_2.10-2.2.1.jar
slf4j-api-1.7.10.jar
slf4j-log4j12-1.7.10.jar
snappy-java-<IP_ADDRESS>.jar
spark-bagel_2.10-1.4.0-SNAPSHOT.jar
spark-catalyst_2.10-1.4.0-SNAPSHOT.jar
spark-core_2.10-1.4.0-SNAPSHOT.jar
spark-graphx_2.10-1.4.0-SNAPSHOT.jar
spark-launcher_2.10-1.4.0-SNAPSHOT.jar
spark-mllib_2.10-1.4.0-SNAPSHOT.jar
spark-network-common_2.10-1.4.0-SNAPSHOT.jar
spark-network-shuffle_2.10-1.4.0-SNAPSHOT.jar
spark-repl_2.10-1.4.0-SNAPSHOT.jar
spark-sql_2.10-1.4.0-SNAPSHOT.jar
spark-streaming_2.10-1.4.0-SNAPSHOT.jar
spire-macros_2.10-0.7.4.jar
spire_2.10-0.7.4.jar
stax-api-1.0.1.jar
stream-2.7.0.jar
tachyon-0.5.0.jar
tachyon-client-0.5.0.jar
uncommons-maths-1.2.2a.jar
unused-1.0.0.jar
xmlenc-0.52.jar
xz-1.0.jar
zookeeper-3.4.5.jar

Closing this as it's superceded by #6617 and upcoming follow-up PRs.


prusa-slicer: fix binary name on darwin

Description of changes
Supersedes #207568.
Things done

Built on platform(s)

[ ] x86_64-linux
[ ] aarch64-linux
[ ] x86_64-darwin
[x] aarch64-darwin

[x] For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
[ ] Tested, as applicable:

NixOS test(s) (look inside nixos/tests)
and/or package tests
or, for functions and "core" functionality, tests in lib/tests or pkgs/test
made sure NixOS tests are linked to the relevant packages

[ ] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
[x] Tested basic functionality of all binary files (usually in ./result/bin/)
23.05 Release Notes (or backporting 22.11 Release notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module
[ ] (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes

[x] Fits CONTRIBUTING.md.

Merged https://github.com/NixOS/nixpkgs/pull/207568 since it seemed a less hacky approach.


[Feature Request] Unable to Unload or Swap adapters at runtime

Currently, adapters can be loaded with:
model.load_weights('/path/to/weights', strict=False)

However, there is no way to either unload the weights:
model.unload_weights()

or to swap in a new one dynamically:
model.swap_weights('/path/to/other/weights')

This is useful for several use cases, including DPO loss calculation & Dynamically serving LoRA Adapters

You can swap adapters like so:
model.load_weights("adapters1.safetensors"), strict=False)

# Swap to new adapters
model.load_weights("adapters2.safetensors"), strict=False)

For unloading weights.. it really depends what you want to do. If you just want to delete the entire model.. simply delete it:
del model

Though maybe for that you could say more about the use case..

Sweet.  For unloading, what I had in mind was for making the combination of using model.load_weights(..) followed by model.unload_weights(..) an idempotent operation.  So, I can run LoRA on a model for a bit (making a new adapter), then run some evaluations comparing results from the original model vs. the adaptation and continue that way until the LoRA is completed.


CreateChatCompletionWithFunctionsAsync Failing

Describe the bug
I'm trying to run the CreateChatCompletionWithFunctionsAsync test as I can't get the function feature to work in my own code, but the same error is occurring with the test.
It fails on the response containing the tool message.
To Reproduce

Run CreateChatCompletionWithFunctionsAsync  test against OpenAI
Notice exception occurs on second ( response = await request.ExecuteAndCalculateCostAsync(); )

Error:
{
"error": {
"message": "Invalid parameter: messages with role 'tool' must be a response to a preceeding message with 'tool_calls'.",
"type": "invalid_request_error",
"param": "messages.[1].role",
"code": null
}
}
Code snippets
No response
OS
Windows
.Net version
.Net 8
Library version
Latest master

In the new version 4.0.0 this issue is solved.


Why not use weak reference to subscriber property in Subscription?

If use weak reference to subscriber in Subscription class, it seems application do not need invoke "unregister" method. This is convenient for app when the subscriber is a Activity/Fragment/Service...

I would be against this because WeakReferences can cause very problematic issues in cases where you instantiate an anonymous class. Take for instance:
mEventBus.register(new Subscriber() {
    public void onEvent(Event event) {
           // do something
    }
});

Since nothing would then hold a strong reference to the subscriber it could potentially be garbage collected.
Note: anonymous inner class hold implicit references to their outer class but not the otherway around unless you hold reference to the anonymous inner class in a member variable.


dictionaryFile does not work

I migrated from osxappbundle-maven-plugin but my dictionaryFile seems to no longer work.
My pom.xml has
<dictionaryFile>${basedir}/src/main/app-resources/Info.plist</dictionaryFile>

and it used to work, but now I get
Could not find resource for template /Users/chen/maven/chatter/src/main/app-resources/Info.plist

I've put my custom dictory into src/main/resources/my/Info.plist.template and configured my/Info.plist.template
But I constantly get
[ERROR] Failed to execute goal io.github.appbundler:appbundle-maven-plugin:1.0-SNAPSHOT:bundle (default) on project XXX: Could not find resource for template my/Info.plist.template: Unable to find resource 'my/Info.plist.template' -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal io.github.appbundler:appbundle-maven-plugin:1.0-SNAPSHOT:bundle (default) on project XXX: Could not find resource for template my/Info.plist.template
I've tried to debug it but failed at
velocity.getEngine().mergeTemplate(dictionaryFile, "UTF-8", velocityContext, writer);
Please help.

@cpesch
Please run mvn clean && mvn compile and check if your Info.plist.template  exists in target/classes. (perhaps the file located target/classes/my/Info.plist.template)
This plugin searches Info.plist in target/classes and apply Velocity templating.

dir .\XXX\target\classes\my\Info.plist.template

Directory: XXX\target\classes\my

Mode                LastWriteTime     Length Name

-a---        02.02.2015     09:49       1560 Info.plist.template
As you can see, the file exists. And the error message is still the same: Unable to find resource 'my/Info.plist.template'
Your documentation is pretty clear about this, but it simply doesn't work for me. Is it because I work with Windows and you're using Linux?

[INFO] --- appbundle-maven-plugin:1.0-SNAPSHOT:bundle (default) @ XXXX ---
Checking for additionalBundledClasspathResources: null
Feb 02, 2015 1:14:37 PM org.apache.velocity.runtime.log.JdkLogChute log
INFO: FileResourceLoader : adding path 'target/classes'
[ERROR] ResourceManager : unable to find resource 'my/Info.plist.template' in any resource loader.

OK. I got it.
This cause is Windows. Windows path deliminator is \, not /.
I will fix soon. Sorry.

@cpesch Probably I fixed with 17897bd. Please re-try.

Thank you for your quick help, but just the path separators don't seem to be the problem:
[INFO] --- appbundle-maven-plugin:1.0-SNAPSHOT:bundle (default) @ XXX ---
Checking for additionalBundledClasspathResources: null
Feb 02, 2015 4:56:02 PM org.apache.velocity.runtime.log.JdkLogChute log
INFO: FileResourceLoader : adding path 'target\classes'
[ERROR] ResourceManager : unable to find resource 'my/Info.plist.template' in any resource loader.
How can I debug where Velocity is searching its templates?

I observed a problem during debugging:
File f = new File(TARGET_CLASS_ROOT, dictionaryFile);
When I call f.getAbsolutePath() it points to the directory of the root pom.xml (XXX\target\classes\my\Info.plist.template) and not the directory of the module XXX\XXX-for-MacOSX\target\classes\my\Info.plist.template.
When I copy my Info.plist.template to XXX\target\classes\my\Info.plist.template it works.

Thanks for feedback.
I have not checked with sub-moduled maven projects.
Could you create a new issue about it? Because it's getting off this issue.


fix: calculate trace hash from both class ID and token ID

Description
Fix for feat/ics721-impl
The class ID and the token ID specify the NFT.
The trace hash should be calculated from both IDs, and we need to submit a trace event per each token.

PR author checklist:

[ ] Added changelog entry, using unclog.
[ ] Added tests.
[ ] Linked to GitHub issue.
[ ] Updated code comments and documentation (e.g., docs/).
[ ] Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

[ ] Reviewed Files changed in the GitHub PR explorer.
[ ] Manually tested (in case integration/unit/mock tests are absent).

Just a nit. To pass the CI we need to place context, handler, and module modules under the #[cfg(feature = "serde")] flag

Does this PR mean that the same event within the bianjieai/nft-transfer repo should get fixed as well?

I think so. In bianjieai/nft-transfer, the prefixed class is handled like the prefixed denom in ICS-20. However, specifying the actual token is required with not only the class but also the token ID. In Cosmos, a non-NFT token received over IBC is shown as ibc/{hash}. I believe it's natural to express both non-NFT and NFT as ibc/{hash}."

Does this PR mean that the same event within the bianjieai/nft-transfer repo should get fixed as well?

I think so. In bianjieai/nft-transfer, the prefixed class is handled like the prefixed denom in ICS-20. However, specifying the actual token is required with not only the class but also the token ID. In Cosmos, a non-NFT token received over IBC is shown as ibc/{hash}. I believe it's natural to express both non-NFT and NFT as ibc/{hash}.

Makes sense. Thank you!


Added conditional function and ability to output additional data

Added a conditional function that allows one to control the amount of output received.
Also added the ability to pass in additional data to be printed - this is mostly useful with timeit_section and is pretty much irrelevant with timeit_function.

@kkctor can you please have a look into failing CI ?

@kkctor can you please have a look into failing CI ?

That is fixed in the latest commits.
It was just an indentation issue that I didn't notice.
You will need to run the workflow to let me know if there's any other similar place that needs to be fixed

Yeah, there is this error:
./ptimeit/_impl.py:49:21: E225 missing whitespace around operator

Btw, you should be able to run those checks locally too, check out these steps from CI.

Yeah, there is this error:
./ptimeit/_impl.py:49:21: E225 missing whitespace around operator

Btw, you should be able to run those checks locally too, check out these steps from CI.

Need to do isort . -rc --check-only doesn't find anything without -rc.

@kkctor would be nice to add a test case for these changes.

https://github.com/narekgharibyan/ptimeit/pull/1/commits/bdeb02a65709e4e4568bdfb8f1c10322c6b9cd5a

@kkctor thanks for your contribution.
This is already published in v0.1.2 release.


Task level user setting looks not working, docs mistake

Hello,
Noticed that in docs section https://spot.umputun.dev/#playbook-types said that I can specify ssh_key:

The simplified playbook does not support task-level on_error, user, and ssh_key fields, while the full playbook does. See the Task details section for more information.

But link leads to https://spot.umputun.dev/#tasks-and-commands, where it is said that only 3 of 4 fields are available: on_error, user, targets.
Actually I am trying to config my playbook to have 2 tasks, each on different remote. Here is playbook:
ssh_key: ~/.ssh/id_rsa

tasks:
  - name: t1
    user: "user1"
    targets:
      - "host2"
    commands:
      - name: test
        script: echo hello, world!

  - name: t2
    user: "user2"
    targets:
      - "host2"
    commands:
      - name: test
        script: echo hello, world!

But it is not working. If I specify user=user1 on playbook level, then t1 is working and t2 crashes with
failed, can't run task "t2" for target "***": 1 error(s) occurred:
   [0] can't connect to ***: failed to create client connection to ***:22: ssh: handshake failed: ssh: unable to authenticate, attempted methods [none publickey], no supported methods remain

Obviously, because of wrong user selection
Also it is not clear if ssh_key is supported on task level?
Currently my workaround is to have 2 separate playbooks for each host

Maybe I am wrong, but here looks like the root of the problem:
https://github.com/umputun/spot/blob/c9fd18516ee32ca111afb1ab1fcc893f38489eb9/pkg/runner/runner.go#L91
should it be tsk.User, rather than host.User?

just to clarify if this is not a typo in your playbook before I go any deeper - you said "... playbook to have 2 tasks, each on different remote" however the playbook you provided runs both tasks on the same host2

Yup, this is a typo, I meant host1 and host2, of course

so, I would assume it fails with a correct host, right? If this is the case, I'll take a look.
in the meantime, you can specify top-level targets and set the user here, i.e.
targets:
  dev:
    hosts:
      - {name: "dev1", host: "dev1.example.com", port: 22, user: user1}
      - {name: "dev2", host: "dev2.example.com", port: 22, user: user2}

tasks:
  - name: t1
    targets:
      - "dev1"
    commands:
      - name: test
        script: echo hello, world!

  - name: t2
    targets:
      - "dev2"
    commands:
      - name: test
        script: echo hello, world!

it works, thanks!


adding python version of regrid.pl

Add python version of regrid wrapper.  Hopefully it will replace regrid.pl . More testing is needed.
Requires related PR: https://github.com/GEOS-ESM/GEOSgcm_GridComp/pull/571

@weiyuan-jiang I might want to work on you with this to make it more "in-line" with the Perl version. I made some updates recently in there and it would be nice to make this roughly the same.

@weiyuan-jiang, @mathomp4 when we meet on Monday, would you please walk me through this so I can add in an option to let user pick path to his/her boundary conditions, tile file etc?

I would like to add that option, test for my use case
If you are okay, push it to this branch

Thanks!

@weiyuan-jiang, @mathomp4 when we meet on Monday, would you please walk me through this so I can add in an option to let user pick path to his/her boundary conditions, tile file etc?
1. I would like to add that option, test for my use case

2. If you are okay, push it to _this_ branch

Thanks!

@sanAkel , Yes, I can help add those options to regrid.py. We would like your suggestion to improve this tool

Changing to "draft" PR for now.  FYI, the land group, with Weiyuan's support, will attempt to clean up the land portion of the mk_restarts package in the GCM GC repo, which includes loads of duplicate code and is in dire need of restructuring.  The mk_restarts effort and the present PR need to be synchronized.  This will likely take a while longer.

Changing to "draft" PR for now. FYI, the land group, with Weiyuan's support, will attempt to clean up the land portion of the mk_restarts package in the GCM GC repo, which includes loads of duplicate code and is in dire need of restructuring. The mk_restarts effort and the present PR need to be synchronized. This will likely take a while longer.

@gmao-rreichle may be we can proceed with whatever is in current format and attempt:

will attempt to clean up the land portion of the mk_restarts package in the GCM GC repo, which includes loads of duplicate code and is in dire need of restructuring.

The reason for suggesting :arrow_up: is to get the code PR "merged" via @sdrabenh or I am afraid, this will take longer ... meaning, @GEOS-ESM/land-team's clean up of mk_restarts, then consideration by @sdrabenh. This is just a suggestion!

Changing to "draft" PR for now. FYI, the land group, with Weiyuan's support, will attempt to clean up the land portion of the mk_restarts package in the GCM GC repo, which includes loads of duplicate code and is in dire need of restructuring. The mk_restarts effort and the present PR need to be synchronized. This will likely take a while longer.

@gmao-rreichle may be we can proceed with whatever is in current format and attempt:

will attempt to clean up the land portion of the mk_restarts package in the GCM GC repo, which includes loads of duplicate code and is in dire need of restructuring.

The reason for suggesting ⬆ is to get the code in this present PR "merged" via @sdrabenh or I am afraid, this will take longer ... meaning, @GEOS-ESM/land-team's clean up of mk_restarts, then consideration by @sdrabenh. This is just a suggestion!

@sanAkel: I'm a bit unclear what exactly you're suggesting here.  If you're suggesting that we merge the new python version of regrid, my concern would be that it's going to be very difficult to test all possible use cases, of which there are a lot for land restarts.  Having the new  python version tied into the land-related mk_restarts cleanup would take care of this issue.  And yes, this is going to take time, unfortunately.  At the moment, I wouldn't quite know how to approve this PR for the land team (whose approval isn't actually required...).  We can of course discuss how progress on this PR relates to development needs by the ocean and/or landice teams.  It's not my decision, of course.  Just my 2c.

very difficult to test all possible use cases, of which there are a lot for land restarts.

I was not aware of such test cases! Since they exist, I support holding this PR until ALL possible tests have been made.
While I say this, I would suggest that:

The modifications/additions within this PR be tested, so as to make sure ALL tests are passing - incl. land restarts.
Once they pass, any clean up can happen, later.

To be more clear:

If tests in (above) 1 show that this code in this PR is failing, obviously, this PR remains in draft form!
IMO I was hoping for an alternative to the regrid.pl through this python based handler. It seems to be working for stock test case(s); I had no idea about all land restarts!
Holding this work for 💯 % perfection can be avoided!

I see no required approver list (names) or number of approvals: 1 seems enough!! So all the :arrow_up: is just a request! 😄

very difficult to test all possible use cases, of which there are a lot for land restarts.

I was not aware of such test cases! Since they exist,...

The problem is, they do not (yet) exist.  If they existed, I'd be happy to test and merge.  Right now, I'm not even sure we know use cases should work.  I certainly don't know.

@gmao-rreichle I think I can see multiple issues. Please see above "request" that should be simple step to disengage things ... and honestly think its necessary!

@weiyuan-jiang checking here ... any ETA when this can be un-drafted?

I will speed it up

I will speed it up

🙏

@weiyuan-jiang There appear to be a few unresolved issues in this PR that are on your plate.  Can you please summarize the status of each and provide an estimate for when you might be able to complete them?  Thanks.

Tom, I tested one or two cases and it seems working. What I need is more tests. Without testing and benchmark, I don't have much confidence...

There appear to be a few unresolved issues in this PR that are on your plate. Can you please summarize the status of each and provide an estimate for when you might be able to complete them? Thanks.

@tclune: I am working with @weiyuan-jiang on this and a related PR in the GEOSgcm_GridComp.  The necessary changes are quite complex, owing to the current, messy state of the mk_restarts package.  Alas, I don't expect this to get done quickly, given the complexity and other, competing priorities.

@sdrabenh Hi, Scott, do you have bench cases of regrid.pl ? I would like to test our regrid.py . @gmao-rreichle @biljanaorescanin

@rtodling and or @gmao-jstassi  may have some ... as well.

After talking with @tclune, I think we should proceed as follows:

Think of regrid.py as a new tool that should be released ASAP.  Besides the new script being in python, it also interfaces with new/revamped Fortran programs in the GEOSgcm_GridComp  Surface/Utils/mk_restarts directory.  (Hence the slower-than-hoped-for progress on this PR.)
The existing regrid.pl should not be touched, nor should there be changes in any of the associated old Fortran programs in the GEOSgcm_GridComp  Surface/Utils/mk_restarts directory.  This ensures full compatibility with all existing setup and regridding scripts.  (The ultimate goal is to phase out the old regrid.pl and its underlying old Fortran programs.)
As of right now, regrid.py was successfully tested by @weiyuan-jiang for re-gridding (re-tiling) between a couple of cube-sphere grid resolutions and between a couple of EASE grid resolutions.  @biljanaorescanin still needs to test regrid.py when the bcs version changes (e.g., from Icarus to Icarus-NLv3, as in MERRA-2 to FP 5.25+) without and with also changing the resolution at the same time.  This will test parts of the code (Catchment prognostics rescaling) that have not yet been tested.  With any luck, this can be done within a week or so.
Once the land group is satisfied that regrid.py works for the above key use cases, we will ask @sdrabenh to do his testing of the PR (mostly verifying that regrid.pl indeed remains intact), merge the PR, and create a new tag.
Next, we can solicit input from across GMAO on re-gridding (re-tiling) use cases, which can then be transitioned to regrid.py with the support of the SI team.
Eventually, we can remove regrid.pl and its old Fortran programs from the develop/main branch.

Feel free to chime in and correct the above as needed.
cc: @tclune @sdrabenh @sanAkel @mathomp4 @biljanaorescanin @weiyuan-jiang @rtodling @gmao-jstassi

Thanks @weiyuan-jiang , I will try again and let you know how it goes.

The program search restart files under
rst_dir/upperair
rst_dir/surface
rst_dir/analysis
It may not be convenient. Should we just leave all restart files under  rst_dir/  without subdirectories?

The program search restart files under
rst_dir/upperair rst_dir/surface rst_dir/analysis
It may not be convenient. Should we just leave all restart files under rst_dir/ without subdirectories?

Either that or to list what each subdirectory has to have from the restart files. I doubt many users will know what to copy where if we don't list files.

@weiyuan-jiang I tried running this new script interactively and encountered the error below after the questionary section:
input tile file: /discover/nobackup/ltakacs/bcs/Icarus-NLv3/Icarus-NLv3_MERRA-2//CF0180x6C_DE1440xPE0720/CF0180x6C_DE1440xPE0720-Pfafstetter.til

output tile file: /discover/nobackup/ltakacs/bcs/Icarus-NLv3/Icarus-NLv3_Reynolds//CF0024x6C_DE0360xPE0180/CF0024x6C_DE0360xPE0180-Pfafstetter.til
Traceback (most recent call last):
  File "/discover/nobackup/projects/gmao/g6dev/sdrabenh/Models/GIT_PRs/PRs_20220517/install/bin/regrid.py", line 62, in <module>
    main(sys.argv[1:])
  File "/discover/nobackup/projects/gmao/g6dev/sdrabenh/Models/GIT_PRs/PRs_20220517/install/bin/regrid.py", line 41, in main
    params = regrid_params(config)
  File "/gpfsm/dnb78s1/projects/p18/sdrabenh/Models/GIT_PRs/PRs_20220517/install/bin/regrid_params.py", line 41, in __init__
    with  open('regrid_params.tpl', 'r') as f:
FileNotFoundError: [Errno 2] No such file or directory: 'regrid_params.tpl'

I confirm this path/file does exist, but the script cannot locate it internally. Is there something special that needs to be loaded first?

@weiyuan-jiang @tclune @gmao-rreichle @sanAkel @biljanaorescanin I have a few comments about the general flow of this new python script compared to the old pearl script after trying it. My first observation is that all the input questions appear to be grouped together, followed by all the output questions. For example, in the old pearl script:
...
INPUT atmosphere grid:
Enter OUTPUT atmosphere grid:
INPUT atmosphere levels:
Enter OUTPUT atmosphere levels:
...

This isn't necessarily a bad thing, but quite different than what most users are accustomed to. I'm just curious what motivated that change?
Secondly, in the modern age of netcdf and python, why can't we automatically read the restart files to know exactly what the input resolutions, data, etc. are? Even the old pearl script filled in the atmosphere grid resolution, but this new script does not. There are additional questions such as Enter input restarts expid: - is this necessary? Also, the Enter new restarts expid: is not pre-filled as the pearl script did. It seems to be an unnecessary burden to the user to fill in all these prompts. I fully admit I have not been part of the development discussion, so there could be a good reason for this. However, I would love to see less questions and more automatic detection/pre-filling for the interactive side of this script.

Good points, @sdrabenh, and thanks for test driving.  Re. the auto-detection of the grid in input restarts: This may not work for the tile-based restart files, not sure how much info we have in, say, Catchment restarts.  Ideally, of course, we don't have to ask.
Also, based on the snippet posted by @sdrabenh above, it looks like the user is not provided a sample of possible grid specs.  How would we tell the script that the (input or output) rst file is/should be in the EASEv2_M09 tile space?  "EASEv2_M09"?  "M09"?  Some other string?  For make_bcs, we offer a list of choices in the questionnaire.

Thanks for the comments. Will address your concern

why can't we automatically read the restart files to know exactly what the input resolutions,

My original thought was that the users don't need "fvcore_internal_rst" to do regridding of other restart files. But  we can make it more specific to gcm restarts if users go through questionnaire.  We have a verification though if the users input wrong input grid.

I have fixed this. Now the program should be able to run from different locations as long as regrid_params.py and regrid_params.tpl stay in the same location. I have fixed a few other bugs too.

why can't we automatically read the restart files to know exactly what the input resolutions,

My original thought was that the users don't need "fvcore_internal_rst" to do regridding of other restart files. But we can make it more specific to gcm restarts if users go through questionnaire. We have a verification though if the users input wrong input grid.

@weiyuan-jiang I'm not sure I understand your original thoughts, but I often use regrid.pl for changing vertical/horizontal resolution for the atmosphere/ocean from various sources for my gcm experiments. So I think in terms of the gcm. I agree that auto-detection may not be feasible for every question, tile files, or even for binary restarts (shudder). However, I'm a big proponent of using as much automation as possible rather than a straight-up list of blank questions. I have seen many users stumble with regridding because they don't know their source files. At the very minimum, if a user provides an input directory containing a single set of modern Jason/Icarus restarts, python commands could rather easily extract the EXPID, date, time, levels, atmospheric grid, etc. I would suspect there might be some key variables identifying the catchment or ocean types too - but admittedly that isn't my wheelhouse. For me, the ideal would be to have the script interrogate all the input files and list everything that could be identified for a user to confirm with a single "y", or alternatively go back and change an option if they knew otherwise. I suspect that might be difficult to implement at this point.

I have fixed this. Now the program should be able to run from different locations as long as regrid_params.py and regrid_params.tpl stay in the same location. I have fixed a few other bugs too.

Thanks! That is great. I will give it a try

@sdrabenh , That is a good point. I will let the program detect the information as much as possible. For example, it only asks for resolution if it could not find fvcore restart. It won't be hard to make such changes.

I tested against mk_GEOSldasRestarts.F90 and it is zero-diff. But it is none zero-diff against mk_CatchRestart.F90 which is used by regrid.pl.  The non-zero diff variables are TSURF, TC(:,3), QC(:,3), CM, CH, FR, CQ . Those variable are re-set in mk_GEOSldasRestarts.F90 when regridding.  Sarith once told me mk_GEOSldasRestarts.F90 is more advanced comparing with mk_CatchRestarts.F90.  How do you think of those variables? I can make it zero-diff easily if you like.  @gmao-rreichle

I tested against mk_GEOSldasRestarts.F90 and it is zero-diff. But it is none zero-diff against mk_CatchRestart.F90 which is used by regrid.pl. The non-zero diff variables are TSURF, TC(:,3), QC(:,3), CM, CH, FR, CQ . Those variable are re-set in mk_GEOSldasRestarts.F90 when regridding. Sarith once told me mk_GEOSldasRestarts.F90 is more advanced comparing with mk_CatchRestarts.F90. How do you think of those variables? I can make it zero-diff easily if you like. @gmao-rreichle

@weiyuan-jiang : Here are a couple of thoughts:

The variables in question have little memory and spin up quickly, so small differences should be of little practical concern.
TSURF should not be needed in the catch restart file because it is not a Catchment model prognostic variable.  I'm fairly sure that GEOSldas does not use it.  It is possible that the GCM or other apps rely on TSURF being in the catch restart file.
I don't know how TSURF, TC, QC, ... are in modified in mk_GEOSldasRestarts.F90 but not in mk_CatchRestart.F90s.  Can you point us to the exact place in the code where the differences originate?
Assuming the modified TSURF, TC, QC, ... from mk_GEOSldasRestarts.F90 are more appropriate, then we would have to discuss with @sdrabenh and others whether it's preferable to include these changes in the regrid.pl perl script or whether it is preferable to maintain 0-diff for regrid.pl.  (We should definitely use the more appropriate treatment with the new regrid.py python script.)
I remember @biljanaorescanin saying that some temperatures end up with 0 values when you get non-zero-diff with regrid.pl.  If this is indeed the case, then it would need to be fixed.  Earth system temperatures cannot be 0.  Presumably, this would be an issue with no-data-value handling.

I remember @biljanaorescanin saying that some temperatures end up with 0 values when you get non-zero-diff with regrid.pl. If this is indeed the case, then it would need to be fixed. Earth system temperatures cannot be 0. Presumably, this would be an issue with no-data-value handling.

from output catch restart using .py code :
    -1 :       Date     Time   Level Gridsize    Miss :     Minimum        Mean     Maximum : Parameter name
   46 : 2022-05-16 21:00:00       0  4752080       0 :      0.0000      216.99      327.52 : TC
from output catch restart  using .pl code:
   46 : 2022-05-16 21:00:00       0  4752080       0 :      238.61      285.21      327.52 : TC
units for TC are kelvins so min zero with .py code is the issue.

In mk_CatchRestarts.F90, every variable is re-tiled. But in mk_GEOSldasRestats.F90,  Some variables are reset:
https://github.com/GEOS-ESM/GEOSgcm_GridComp/blob/3e52659a4baa55aea5b0c937afcbfe90f0fafd92/GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSsurface_GridComp/Utils/mk_restarts/mk_GEOSldasRestarts.F90#L2985-L3002
TC and QC only go to the first 3 : ( maybe it is a bug by Sarith? I am not sure)
https://github.com/GEOS-ESM/GEOSgcm_GridComp/blob/3e52659a4baa55aea5b0c937afcbfe90f0fafd92/GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSsurface_GridComp/Utils/mk_restarts/mk_GEOSldasRestarts.F90#L2835-L2869
TSURF is set to zero in catchcn ( I can correct it in catch model)
https://github.com/GEOS-ESM/GEOSgcm_GridComp/blob/3e52659a4baa55aea5b0c937afcbfe90f0fafd92/GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSsurface_GridComp/Utils/mk_restarts/mk_GEOSldasRestarts.F90#L1063

In mk_CatchRestarts.F90, every variable is re-tiled. But in mk_GEOSldasRestats.F90, Some variables are reset:

My suspicion is that mk_GEOSldasRestats.F90 makes up WW, FR, CH, CM, CQ so that these variables are present in the restart in case the restart is used for the GCM, which needs these variables.  (GEOSldas does not need them.)  Before GEOSldas, LDASsa created catch restart files that did not have these variables, so if someone wanted to use an offline (LDASsa) restart for the GCM, they had to add these variables into the restart.  The made-up values are probably there for this reason. It's odd (but entirely surprising) that these variables are made up even if present in the input catch restart.
Going forward, these variables should not be made up.  GEOSldas can create restarts that include these variables.  So regridding should use these variables from the input restart.  If they are missing from the input restart, they should be skipped (set to no-data, or just not included in the catch restart).   For the regrid perl script, this (probably) means that it should use mk_CatchRestart.F90 to maintain 0-diff.  For the regrid python script, the corresponding new F90 program in GEOS Catch GC should also not make up WW, FR, CH, CM, CQ.

TC and QC only go to the first 3 : ( maybe it is a bug by Sarith? I am not sure)

TC(4,:) and QC(4,:) are the temp and spec humidity over snow, which are diagnosed from the snow water equivalent, depth, and heat content.  In this sense, they are like TSURF and not model prognostic variables.  GEOSldas does not need TC(4,:) and QC(4,:) in the catch restart.  I assume the same is true for the GCM, but I can't say for sure.

TSURF is set to zero in catchcn ( I can correct it in catch model)

Not sure why this block sets variables to 0 rather than no-data.  It looks like these 0s are immediately written into the restart file.  I might be missing something here.
cc: @rdkoster @sanAkel @sdrabenh @biljanaorescanin

I made changes to https://github.com/GEOS-ESM/GEOSgcm_GridComp/tree/feature/rreichle/mk_restarts_cleanup  . Let's satisfiy GCM's zero-diff first. @biljanaorescanin It should be zero-diff now even for those "not important" variables.

@weiyuan-jiang I am still getting runtime failures regridding. Please see an example interactive case in my directory. The complete logfile with inputs is located here: /discover/nobackup/projects/gmao/g6dev/sdrabenh/test_exps/PRs_regrid.py/test1_out_C360_cs_python/output.log
The complete log for the pearl script doing the exact same thing is here:
/discover/nobackup/projects/gmao/g6dev/sdrabenh/test_exps/PRs_regrid.py/test1_out_C360_cs_pearl/output.log
The pearl script works but the python script does not. Also, the python script should recognize whether the user is on an interactive node a determine whether to submit a batch job accordingly. The pearl script is able to do this.
@tclune I do not think this script is ready yet

I know the problem here. 1) need to convert the relative path of restart dir to absolute path.  2) I can add the detection of interactive mode.

@gmao-rreichle It's been more than the 1-2 weeks for testing since we last talked. Can you please summarize again here, what the remaining obstacles are and a reasonable timeline for acceptance for the initial release.

@tclune : The land group has been testing this PR since the two of us talked.  Unfortunately, testing turned up a few issues that need fixing.  The latest delay is because different results were produced depending on which of the underlying (existing) Fortran programs was connected to regrid, which pointed to a flaw in the (new) regrid python version.  The discrepancy was inherited from the previous author of the tools.  @weiyuan-jiang cannot be expected to understand the science issues, and he originally ended up with the wrong choice for regrid.py.  The good news is that the process is working, albeit at a snail's pace.  This is unfortunate, but the necessity to resolve this PR "by committee" just takes more time than we want it to take.
The alternative is to merge code that we have not fully verified, which I am reluctant to do.  I am sorry, but not surprised, that this PR is taking a lot longer to resolve than anticipated.  I had communicated this expectation from the start.  Unfortunately, this does not bode well for resolving the required additional fixes that are needed in make_bcs.
@rdkoster @biljanaorescanin @sanAkel

OK - we have some serious process problems on the development of this tool.      I should have pushed harder for a minimum test suite when we (SI Team) decided to take this on.   In the mean time we have a PR  that now encompasses multiple bug reports and feature requests.     In a more perfect world, each of those should be their own issue, and we can then resolve them as we go.
So 1st step - I'm going to open a new issue to define a minimal test suite, and notify the major players here.    Possibly this is something @gmao-rreichle et al already have, but I'm guessing that their suite is larger than what I envision.    Once the tool can pass this initial test suite, the PR should be merged.   We can then mine this ticket for new issues/features and open corresponding tickets.   (Happy to have others prioritize among them.)

OK - we have some serious process problems on the development of this tool. I should have pushed harder for a minimum test suite when we (SI Team) decided to take this on. In the mean time we have a PR that now encompasses multiple bug reports and feature requests. In a more perfect world, each of those should be their own issue, and we can then resolve them as we go.
So 1st step - I'm going to open a new issue to define a minimal test suite, and notify the major players here. Possibly this is something @gmao-rreichle et al already have, but I'm guessing that their suite is larger than what I envision. Once the tool can pass this initial test suite, the PR should be merged. We can then mine this ticket for new issues/features and open corresponding tickets. (Happy to have others prioritize among them.)

This is a good idea. Thank you @tclune

@tclune @weiyuan-jiang defining a minimal test suite is a bit nebulous with such an all-encompassing tool like this. From my point of view, the new python script should be able to demonstrate comparable results to the pearl script for current (Icarus/Jason) datasets and resolutions. Regarding the functionality of the tool, it should be able to:

Run irrespective of what the working directory is
Resolve all relative or absolute paths correctly
Recognize whether it is running on login or interactive nodes and execute accordingly
Provide the correct/same default values for wemin, wemout, zoom, etc as the pearl script did
Accept command line arguments without requiring a yaml file so it doesn't break existing code such as this
Demonstrate consistent results whether running the script interactively, or with command line arguments, or using a yaml file

From an AGCM point of view, the most important atmospheric resolutions are C90, C180, C360, and C720. I would think that regridding between higher horizontal resolutions would better demonstrate whether the script is working correctly since they inherently contain more information. We frequently regrid between 72, 91, 137, and 181 levels. In fact, Bill's feature branch is aimed toward non 72-level resolutions, thus all those experiments. Lastly, it is common to regrid between the 4 data ocean resolutions based on whether we are running an OPS-like experiment, EMIPs, AMIPs, etc. Again, just seeing a few cases here demonstrating 0-diffness/equivalency with the pearl script would go a long way in establishing confidence this script is working correctly.
My wish list for this script (after meeting minimal requirements) would be to:

Automatically determine as much information about the input datasets as possible and pre-fill the defaults. In an ideal world the user shouldn't have to know anything about the input dataset, although I'm sure there are limitations to this.
Output the *.CMD text file the pearl script did. This was helpful.
When running interactively, provide the operation summary for the user to okay just like the pearl script did
Add back the coupled ocean resolutions - @sanAkel thoughts here?

@weiyuan-jiang I just tried your python script again and noticed you addressed several issues I mentioned above. Thanks! Improvements already. However, it is still crashing running the same test as yesterday. It got farther but it looks like another missing file issue. Please see my full log here:
/gpfsm/dnb78s1/projects/p18/sdrabenh/test_exps/PRs_regrid.py/test1_out_C360_cs_python/output.log

@sdrabenh - moving your comments above to the issue rather than the PR

@sdrabenh
I addressed most of your comments, except

Command line. I will leave this feature for the future. It seems to me yaml config  file can be treated as command line inout.
ocean coupled model. I am not sure of the logic. The coupled model would show up if MOM5 or MOM6 is chosen. Is that right ? @sanAkel

ocean coupled model. I am not sure of the logic. The coupled model would show up if MOM5 or MOM6 is chosen. Is that right ?

that's right @weiyuan-jiang

Some of you are aware ... but in case your memory needs to be refreshed!
We have been working (@rtodling and I) to add a way to use atmos-only restarts in coupled mode. @mathomp4 kindly made this possible. What that meant is that following option to re-grid was added:

input: data ocean
output: MOM 5 or MOM 6 ocean

This feature must be preserved.
@weiyuan-jiang please note. Thanks!

@sanAkel Thanks for the confirmation.  I think regrid_py preserves that function.

@weiyuan-jiang I just tried your python script again and noticed you addressed several issues I mentioned above. Thanks! Improvements already. However, it is still crashing running the same test as yesterday. It got farther but it looks like another missing file issue. Please see my full log here: /gpfsm/dnb78s1/projects/p18/sdrabenh/test_exps/PRs_regrid.py/test1_out_C360_cs_python/output.log

@weiyuan-jiang I just tried this exact same test case again this morning (3rd time) with your latest updates and I am getting the same error from /discover/nobackup/projects/gmao/g6dev/sdrabenh/test_exps/PRs_regrid.py/test1_out_C360_cs_python.try3/output.log
228-/gpfsm/dnb78s1/projects/p18/sdrabenh/Models/GIT_PRs/PRs_regrid.py/install/bin/mk_LakeLandiceSaltRestarts.x /gpfsm/dnb78s1/projects/p18/sdrabenh/test_exps/PRs_regrid.py/test1_out_C360_cs_python//OutData//CF0360x6C_CF0360x6C-Pfafstetter.til /gpfsm/dnb78s1/projects/p18/sdrabenh/test_exps/PRs_regrid.py/test1_out_C360_cs_python//InData//CF0180x6C_DE1440xPE0720-Pfafstetter.til InData/Jason-3_4_NL_REAMIP_MERRA2_C180.openwater_internal_rst.e19850509_21z.Jason-3_4.Icarus-NLv3_MERRA-2_CF0180x6C_DE1440xPE0720.nc4 0 8
229-Traceback (most recent call last):
230-  File "/discover/nobackup/projects/gmao/g6dev/sdrabenh/Models/GIT_PRs/PRs_regrid.py/install/bin/regrid.py", line 88, in <module>
231-    main(sys.argv[1:])
232-  File "/discover/nobackup/projects/gmao/g6dev/sdrabenh/Models/GIT_PRs/PRs_regrid.py/install/bin/regrid.py", line 77, in main
233-    lls.regrid()
234-  File "/gpfsm/dnb78s1/projects/p18/sdrabenh/Models/GIT_PRs/PRs_regrid.py/install/bin/regrid_lake_landice_saltwater.py", line 105, in regrid
235-    subprocess.call(shlex.split(cmd))
236-  File "/usr/local/other/python/GEOSpyD/4.10.3_py3.9/2022-01-14/lib/python3.9/subprocess.py", line 349, in call
237-    with Popen(*popenargs, **kwargs) as p:
238-  File "/usr/local/other/python/GEOSpyD/4.10.3_py3.9/2022-01-14/lib/python3.9/subprocess.py", line 951, in __init__
239-    self._execute_child(args, executable, preexec_fn, close_fds,
240-  File "/usr/local/other/python/GEOSpyD/4.10.3_py3.9/2022-01-14/lib/python3.9/subprocess.py", line 1821, in _execute_child
241-    raise child_exception_type(errno_num, err_msg, err_filename)
242:FileNotFoundError: [Errno 2] No such file or directory: '/gpfsm/dnb78s1/projects/p18/sdrabenh/Models/GIT_PRs/PRs_regrid.py/install/bin/mk_LakeLandiceSaltRestarts.x'

Please use similar inputs and let me know when you can successfully run this case without errors executing $GEOSUTIL/bin/regrid.py from any arbitrary directory. I will wait to do further testing until I hear from you this case can be successfully run. Also, it looks like the interactive node issue was halfway-addressed based on my output log lines 191-194:
190-/bin/ln -s topo_DYN_ave_360x2160.data topo_dynave.data
191:interactive mode
192-
193-
194-You should have at least 96 cores. Now you only have 1 cores
195-./regrid_upper.j  1>/gpfsm/dnb78s1/projects/p18/sdrabenh/test_exps/PRs_regrid.py/test1_out_C360_cs_python//regrid_upper_log  2>&1

@sdrabenh I am surprised that there is no such file  /gpfsm/dnb78s1/projects/p18/sdrabenh/Models/GIT_PRs/PRs_regrid.py/install/bin/mk_LakeLandiceSaltRestarts.x . I have no access to your folder so I cannot figure that out. Maby different GEOSgcm_GridComp branch ?
In your interactive mode, can you post the result from the command?  " env | grep SLURM_  "
I am wondering you may acquire the nodes in different way.

@sdrabenh I actually had no problem running the tool in all locations and by interactive mode or by sbatch mode.  But I may have different setups from yours.

@sdrabenh I am surprised that there is no such file /gpfsm/dnb78s1/projects/p18/sdrabenh/Models/GIT_PRs/PRs_regrid.py/install/bin/mk_LakeLandiceSaltRestarts.x . I have no access to your folder so I cannot figure that out.

That is very strange. Everyone should have read/execute access to my directories:
drwxr-xr-x 28 sdrabenh g0620   32768 Apr 19 11:10 /gpfsm/dnb78s1/projects/p18/sdrabenh
drwxr-xr-x 42 sdrabenh g0620   32768 May  4 09:26 /gpfsm/dnb78s1/projects/p18/sdrabenh/Models
drwxr-xr-x 25 sdrabenh g0620   32768 Jun  8 21:50 /gpfsm/dnb78s1/projects/p18/sdrabenh/Models/GIT_PRs
drwxr-xr-x 13 sdrabenh g0620   32768 Jun  9 09:48 /gpfsm/dnb78s1/projects/p18/sdrabenh/Models/GIT_PRs/PRs_regrid.py
drwxr-xr-x 13 sdrabenh g0620   32768 Jun  9 09:54 /gpfsm/dnb78s1/projects/p18/sdrabenh/Models/GIT_PRs/PRs_regrid.py/install
drwxr-xr-x  8 sdrabenh g0620   32768 Jun  9 09:54 /gpfsm/dnb78s1/projects/p18/sdrabenh/Models/GIT_PRs/PRs_regrid.py/install/bin
-rwxr-xr-x  1 sdrabenh g0620 1954944 Jun  8 10:37 /gpfsm/dnb78s1/projects/p18/sdrabenh/Models/GIT_PRs/PRs_regrid.py/install/bin/mk_LakeLandiceSaltRestarts

and
drwxr-xr-x 28 sdrabenh g0620 32768 Apr 19 11:10 /gpfsm/dnb78s1/projects/p18/sdrabenh
drwxr-xr-x 11 sdrabenh g0620   512 Jun  7 13:31 /gpfsm/dnb78s1/projects/p18/sdrabenh/test_exps
drwxr-xr-x  7 sdrabenh g0620 32768 Jun  9 10:38 /gpfsm/dnb78s1/projects/p18/sdrabenh/test_exps/PRs_regrid.py
drwxr-xr-x  5 sdrabenh g0620 32768 Jun  9 16:11 /gpfsm/dnb78s1/projects/p18/sdrabenh/test_exps/PRs_regrid.py/test1_out_C360_cs_python.try3

Maby different GEOSgcm_GridComp branch ? In your interactive mode, can you post the result from the command? " env | grep SLURM_ " I am wondering you may acquire the nodes in different way.

I prepended my environment to /gpfsm/dnb78s1/projects/p18/sdrabenh/test_exps/PRs_regrid.py/test1_out_C360_cs_python.try3/output.log

@sdrabenh , Ok, I know what happened. You have not updated GEOSgcm_GridComp. So you did not see .x  executables. Here are my example of interactive run and sbatch run. They both run successfully.
/discover/nobackup/wjiang/f522_interactive
/discover/nobackup/wjiang/f522_out
You got warning message  in interactive mode because you don't have SLURM_NTASKS env variable.  I guess you don't specify --ntasks when acquiring the resource. That is ok, I can add another way to calculate the ntasks.

@sdrabenh , Ok, I know what happened. You have not updated GEOSgcm_GridComp. So you did not see .x executables. Here are my example of interactive run and sbatch run. They both run successfully. /discover/nobackup/wjiang/f522_interactive /discover/nobackup/wjiang/f522_out

Maybe I pulled your branch too early before your fix. I will try again.

You got warning message in interactive mode because you don't have SLURM_NTASKS env variable. I guess you don't specify --ntasks when acquiring the resource. That is ok, I can add another way to calculate the ntasks.

That is correct. Specifying ntasks is not necessary. In my case I asked for nodes instead. Glad you discovered that.

I fixed the line
https://github.com/GEOS-ESM/GMAO_Shared/blob/c114d064cb0e0d6de93c7c4268300161610b791f/GEOS_Util/post/regrid_py/regrid_params.py#L536
@sdrabenh , I am sorry to put the debugging burden on you and thanks a lot for helping me to make this script more robust.

Great thanks. Also, in my random test case #3 not all restarts were produced, but there were no obvious errors in the output log:
/discover/nobackup/projects/gmao/g6dev/sdrabenh/test_exps/PRs_regrid.py/test3_out_C90_o1_python/output.log

@sdrabenh , Sorry again for the bugs.  I made more changes ( please be aware of the changes of script's names)

change regrid.py to remap_restart.py
2)change the default zoom value
3)add support for restarts' file names without time stamp

To not waste your time, would you please give me some sets of results from regrid.pl ? I will do the tests and debugging myself.  Only after I passes all those tests, I would ask your help again.
@biljanaorescanin

@weiyuan-jiang:  I tried to clarify the questions a little bit more.  Let me know if I messed things up.  Here's my commit: https://github.com/GEOS-ESM/GMAO_Shared/pull/238/commits/dd2c48f21b8694aa47f6138f7ea0a168e0937b4b
Also, see my comment about adding a default to the question about the M2 directory.

Hi @weiyuan-jiang, @tclune are there are issues in fixing the errors that @sdrabenh pointed to ⬆. Please see. Thanks!

@sanAkel Not sure where things stand with regard to @sdrabenh issues, but we certainly want them all fixed before wasting more of his time.   Last I checked, @weiyuan-jiang had fixed several of the issues.  He'll have to update us here.

@yvikhlya when time permits, would you please look into issues with MOM5. Thanks!

MOM5 case (if I pick MERRA2 CS to go to MOM5 720x410) : this case will produce .bin catch restart with regrid.pl but not with rebase_restarts.py, Weiyuan said we don’t support catch restarts in .bin format anymore. But it is zero diff between the files.

After all the tests are settled, the PR #272 would re-order the questions

@weiyuan-jiang have you started from a clean build recently? I tested this yesterday and the executable names need to be consistent or else this script fails. It is looking for mk_LakeLandiceSaltRestarts.x but only mk_LakeLandiceSaltRestarts exists. It is looking for mk_catchANDcnRestarts.x but only mk_CatchCNRestarts and mk_CatchRestarts exist. Please be sure all your names are consistent and retest from a clean build.

Talking to @sdrabenh it seems this is a build issue. When I run cases, I run them from install/bin directory and for me all @sdrabenh failed cases were successful, no fails or errors.
For example:
/discover/nobackup/borescan/brisi/test_regrid/try_22/test3_out_C90_o1_python.2
When he runs he uses $GEOSUTIL/bin/remap_restarts.py and somehow has issue with renamed files,  some.x are missing.

@sdrabenh, @biljanaorescanin, @mathomp4:
@weiyuan-jiang added the .x extension to the program executable names for consistency with the rest of GEOS-ESM.  I understand from @biljanaorescanin that the sym-links that @weiyuan-jiang introduced don't work for @sdrabenh when he runs regrid.pl from a place other than install/bin.  If links don't work, we could just keep two copies of the executables, one with .x and one without .x.  The copy without .x would then be discontinued when we discontinue the perl package, presumably later this year.  If this approach is ok, @biljanaorescanin can look into replacing the sym-links with a copy.

@sdrabenh, @biljanaorescanin, @mathomp4: @weiyuan-jiang added the .x extension to the program executable names for consistency with the rest of GEOS-ESM. I understand from @biljanaorescanin that the sym-links that @weiyuan-jiang introduced don't work for @sdrabenh when he runs regrid.pl from a place other than install/bin. If links don't work, we could just keep two copies of the executables, one with .x and one without .x. The copy without .x would then be discontinued when we discontinue the perl package, presumably later this year. If this approach is ok, @biljanaorescanin can look into replacing the sym-links with a copy.

@biljanaorescanin, @weiyuan-jiang, @gmao-rreichle, just to clarify, my "clean" build from checking out this branch on top of main produces an install/bin directory with a mix of executables:
 ls -1 mk_*
mk_CatchCNRestarts
mk_CatchRestarts
mk_CICERestart
mk_GEOSldasRestarts
mk_LakeLandiceSaltRestarts
mk_Restarts
mk_RouteRestarts
mk_runofftbl.x

Most do not have the *.x extension and no sym links exist. When I run $GEOSUTIL/bin/remap_restarts.py interactively from a separate working directory the scripts fail because they are looking for 2 non-existent files .../install/bin/mk_LakeLandiceSaltRestarts.x and .../install/bin/mk_catchANDcnRestarts.x

@sdrabenh but isn't that the problem? My build has that, those sym links.  And all 4 tests are zero diff to pearl.

@sdrabenh but isn't that the problem? My build has that, those sym links.

Correct. Do your sym links exist after building the model or after running remap_restarts.py? Either way, something does not appear to be working correctly and the question is why.


Use 1 consistent word for the same concept in diagnosticMessages.generated.json in ko locale

Fixes #51237
Updated ko translation.

The source of these translations is actually in https://github.com/microsoft/TypeScript/tree/main/src/loc/lcl, which is what should be edited.

Last I checked, we are required to open an issue on an internal microsoft localisation team and copy changes there. I'll see if I can find it.

https://ceapex.visualstudio.com/CEINTL/_workitems/edit/741815 tracks this internally, although it sounds like the translation budget is cut enough that it may not be fixed if it's not deemed high priority.

Unfortunately, the internal bug was declined by the localization team on 15 November 2022.


multi lllm providers?

first of all, thanks for this, I've been testing all the various coding ai assistants and I like this one the best so far as it fits with me and my current vim setup.
One longer term suggestion, claude is the best coder now, but I personally just keep switching out models as needed and I assume something will dethrone claude for programming soon, so maybe start to build in support for other llms later.

That's great to hear!
Right now, Claude is a bit unique thanks to its tool use, but I do expect it to be commoditized. (But TBH also hope Claude gets much better. :)
I think for the future, the way to go is actually something like we do now with the bedrock backend - calling a middleware translator from whatever to Claude API. I'd like to keep the vim plugin itself as simple as possible.

Is there support for openrouter? @pasky

I too wonder if openrouter is supported so I'm looking into it but if someone beats me to it let me know thanks.

I'd be happy to welcome a contribution of Openrouter support, ideally implemented as a Python Claude API emulator like the AWS Bedrock support is.


Fix config folder permissions on create

Fixes #754
What changed?
Updated config directory permissions to allow execute
Why?
Allow directory to function
How did you test it?
Verified locally
Potential risks
Is patch release candidate?
Link to relevant docs PRs

I ended up merging #755 instead and have marked you as a coauthor on that change - thankyou!


fix: streamSSE's doesn't trigger onAbort correctly

Author should do the followings, if applicable

[x] Add tests
[x] Run tests
[x] yarn denoify to generate files for Deno

#1902

Hi, @watany-dev. cc: @yusukebe
I have changed streamSSE from the traditional implementation method to something similar to the original stream helper one, can you review it once as I am not familiar with this implementation?

@sor4chi
Thank you. I was also able to confirm that onAbort stops normally when I actually run it. I think this is the way to go.

@watany-dev Thanks for reviewing!
@yusukebe This PR is review ready, please take a look this!

Hi @sor4chi
Looks good! I've left one comment. Check it please!

fixed!

@sor4chi @watany-dev
Thanks! Merging now.


Please make an isEven()

Otherwise I can only know when a number isOdd, but not when it isEven

const isOdd = (num) => !isEven(num);
const isEven = (num) => !isOdd(num);


Browser version - confusing UX with pod status in sidecar?

Steps to reproduce:

Launch browser version
kubectl get pods -n <namespace>
Select one of your pods, note the pod status in the table.  Click the pod.
Sidecar loads, go to Status tab.
The sidecar briefly shows Pending while KUI is figuring out the pod status.

Pending is a valid phase state in Kubernetes, using Pending as a loading state will be confusing to the user.  Suggest KUI design team use a different loading state behavior instead.

i think this one has been resolved, not ideally, but resolved for now: the sidecar tables are not watch tables. #2234


DecryptException : The payload is invalid

Hello. Package look pretty good, but I have error after start using this. Wherein main functional of package - work fine.
Describe the bug
In clear laravel 5.8 setup and after finish composer require calebporzio/livewire command, i got error in console after running php artisan route:list command.
Error message:  Illuminate\Contracts\Encryption\DecryptException  : The payload is invalid.
To Reproduce
Steps to reproduce the behavior:

Open Terminal on Linux based OS (in my case - Mac OS)
Run command laravel new awesome-project
Run command cd awesome-project
Run command composer require calebporzio/livewire
Run command php artisan route:list
See error

Screenshots
https://www.dropbox.com/s/sc7mqhnwjkboe33/%D0%A1%D0%BD%D0%B8%D0%BC%D0%BE%D0%BA%20%D1%8D%D0%BA%D1%80%D0%B0%D0%BD%D0%B0%202019-08-03%20%D0%B2%2018.58.06.png?dl=0

Guys, i found HttpConnectionHandler class (vendor/calebporzio/livewire/src/Connection/HttpConnectionHandler.php), where we get value from decrypt in getMiddleware function. But if we decrypt NULL value - get DecryptException. In this case i think we need if (request('middleware') !== null) or try {} case () {} construction.
In my local solution i rewrite source code like this:
    // This simulates extending Illuminate/Routeing/Controller
    public function getMiddleware()
    {
        $middleware = [];
        if (request('middleware') !== null) {
            $middleware = decrypt(request('middleware'), $unserialize = true);
        }

        return array_map(function ($m) {
            return [
                'middleware' => $m,
                'options' => [],
            ];
        }, $middleware);
    }

After this php artisan route:list command work fine.

Good catch @maxsprite, and thanks for providing a solution.
However, I think that solution creates a vulnerability in the framework.
I have created my own fixed and pushed it to master. It will be available in the latest release when I tag it. Thanks.


Problem with selecting non-folder tracks

Hello :)
I want an action that selects all non-folder tracks so I can lower the volume of all tracks to increase my headroom without changing the balance, and can't seem to get anything to work reliably.
I have a custom action that is "select all folders" + "toggle (invert) track selection", but that usually leaves me with folder tracks still selected. "Select all non-folders" sounds like it should do exactly what I want, but seems to give completely unexpected results (many non-folder tracks not selected).
Am I doing something wrong, or is there possibly a bug here?
(folder tracks are purple and teal)
http://i.imgur.com/tzfdzBn.gif

I made a macro and it works every time for me:
SWS: Select all folder start tracks
SWS:Toggle (Invert) track seelection
Select all non-folders does nothing here also.

Ah yes! The parents were the problem. I had used those actions based on a recommendation, but it obviously wasn't the right one. Thanks :)


#1518 - Data Type Periods

Checklist

[X] manage.py test passes
[X] tests are included
[X] migrations are included
[X] documentation is changed or added

Affected app(s)

ingest
storage

Description of change
Removed the data tag pattern check for data type tags and changed the field in the database from a CharField to an array of CharFields so we don't care what characters are used for tags and we don't have to do any splitting on a comma separated string.

Ready for review


init() got an unexpected keyword argument 'callbacks'

When I open a .pickle file, it only shows one line of error as the title above.

Thank you for contacting us. Can you please provide me with the file causing the error or provide steps to reproduce it?